Skip to content

Commit

Permalink
changing approach to mapping dicts in encoding and decoding
Browse files Browse the repository at this point in the history
  • Loading branch information
igorapple committed Dec 5, 2024
1 parent 0a15e18 commit 6ef9b26
Showing 1 changed file with 36 additions and 13 deletions.
49 changes: 36 additions & 13 deletions uds/database/text_data_record.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
"""Definition of TextDataRecord which is class for encode and decode values of data records."""
import copy
from typing import Optional, Tuple, Union, Dict
from uu import decode

from uds.database.abstract_data_record import (AbstractDataRecord, DataRecordType, DataRecordPhysicalValueAlias,
DecodedDataRecord)

Expand All @@ -15,7 +18,8 @@ def __init__(self, name: str, length: int, mapping: Dict[int, str] = None) -> No
"""
super().__init__(name)
self.length = length
self.mapping = mapping
# self.__reversed_mapping = None ### should it be deleted?

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

delete

self.__mapping = mapping

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

keep self.mapping = mapping


@property # noqa: F841
def length(self) -> int:
Expand Down Expand Up @@ -67,6 +71,25 @@ def contains(self) -> Tuple["AbstractDataRecord", ...]:
"""Get Data Records contained by this Data Record."""
# TODO

@property
def mapping(self):
"""Get mapping dict."""
return self.__mapping

@mapping.setter
def mapping(self, mapping: dict) -> None:
"""Set the mapping.
:param mapping: dict contains mapping.
"""
self.__mapping = mapping
self.__reversed_mapping = {v: k for k, v in self.__mapping.items()}

@property
def reversed_mapping(self):
"""Get reversed mapping dict."""
return self.__reversed_mapping

def decode(self, raw_value: int) -> DecodedDataRecord: # noqa: F841
"""
Decode physical value for provided raw value.
Expand All @@ -75,13 +98,13 @@ def decode(self, raw_value: int) -> DecodedDataRecord: # noqa: F841
:return: Dictionary with physical value for this Data Record.
"""
if self.mapping:
for k, v in self.mapping.items():
if v == raw_value:
return DecodedDataRecord(name=self.name, raw_value=k, physical_value=raw_value)
raise ValueError("physical_value not found in provided mapping.")
else:
return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value=raw_value)
if self.__reversed_mapping:

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

no need to check if dict exists

try:

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

instead of try, you can check:
if raw_value in self.mapping

Note: mapping is from int to str

decoded_value = self.__reversed_mapping[raw_value]

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

physical_value = self.mapping[raw_value]

return DecodedDataRecord(name=self.name, raw_value=decoded_value, physical_value=raw_value)

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

raw_value is the same as provided to decode method
physical_value is what you just decoded

except KeyError as error:
raise KeyError("raw_value not found in provided mapping.") from error

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

if value is not in mapping, we still want to handle it like a normal raw value:
super(self, RawDataRecord).decode(raw_value (make sure I made no mistake here)

This comment has been minimized.

Copy link
@igorapple

igorapple Dec 7, 2024

Author Collaborator

so i should just left the 84 line instead of handling error, right?

return DecodedDataRecord(name=self.name, raw_value=raw_value, physical_value=raw_value)

def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: # noqa: F841
"""
Expand All @@ -94,10 +117,10 @@ def encode(self, physical_value: DataRecordPhysicalValueAlias) -> int: # noqa:
if isinstance(physical_value, int):
return physical_value
elif isinstance(physical_value, str):
if self.mapping:
for k, v in self.mapping.items():
if v == physical_value:
return k
raise ValueError("physical_value not found in provided mapping.")
if self.__mapping:

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

no need to check if __mapping dict exists.

try:

This comment has been minimized.

Copy link
@mdabrowski1990

mdabrowski1990 Dec 6, 2024

Owner

instead of try-except it is better to just check:
if physical_value in self.reversed_mapping

Note: reversed_mapping is from str to int

return self.__mapping[physical_value]
except KeyError as error:
raise KeyError("physical_value not found in provided mapping.") from error
else:
raise TypeError("During encoding of str mapping must be provided.")

1 comment on commit 6ef9b26

@mdabrowski1990
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look at my remarks

Please sign in to comment.