Made code more DRY #439
Made code more DRY #439
Conversation
|
@jaingaurav : Any comments on this PR ? |
src/diamond/convertor.py
Outdated
| if unit in units: | ||
| # returning self.type_of_unit | ||
| return self.convertb(value, object_type, counter) | ||
| return "None" |
There was a problem hiding this comment.
Please return None vs "None"
There was a problem hiding this comment.
I tried doing that but tests started failing since convertb also returns None. Let me know if you want to have -1 instead of "None" as return value of _object_mapper function.
There was a problem hiding this comment.
When doesn convertb return None?
There was a problem hiding this comment.
@Lacrymology : Just checked in the failing tests, that the issue isn't due to convertb returning None but _object_mapper returning None causes float(actual_value) to throw TypeError. You can check here for more details.
There was a problem hiding this comment.
I don't see how there's a difference between if obj == "None" and if obj is None in L67 &c. 😕
|
Made some minor comments. If you're able to resolve them please squash the commits and I'll check it in. Thanks. |
|
@jaingaurav : I have replied to the comments made by you and squashed the commits. |
5474780 to
66aec5f
Compare
|
Any comments @jaingaurav ? |
src/diamond/convertor.py
Outdated
| if unit in ['yottabit', 'Ybit', 'Yibit']: | ||
| return self.yottabit(value=value) | ||
| obj = self._object_mapper(unit, value, _LIST_OF_UNITS_BIT, self.bit) | ||
| if obj != "None": |
src/diamond/convertor.py
Outdated
| if unit in ['yottabyte', 'YB', 'YiB']: | ||
| return self.yottabyte(value=value) | ||
| obj = self._object_mapper(unit, value, _LIST_OF_UNITS_BYTE, self.byte) | ||
| if obj != "None": |
| if value is None: | ||
| return self.value / 8 | ||
| else: | ||
| self.value = float(value) * 8 |
There was a problem hiding this comment.
add return self.value on next line
| if value is None: | ||
| return source() / pow(1024, offset) | ||
| else: | ||
| source(value * pow(1024, offset)) |
There was a problem hiding this comment.
and change def bit to
def bit(self, value=None):
- if value is None:
- return self.value
- else:
+ if value is not None:
self.value = float(value)
+ return self.value|
Added 6 comments, i verified those changes work |
|
@shortdudey123 : If I replace "None" with None, it doesn't pass the test cases. Please check. |
| # Lists are in the order of increasing magnitude | ||
| _LIST_OF_UNITS_BIT = [['bit', 'b'], | ||
| ['kilobit', 'kbit', 'Kibit'], | ||
| ['megabit', 'Mbit', 'Mibit', 'Mbit'], |
| if unit in ['yottabit', 'Ybit', 'Yibit']: | ||
| return self.yottabit(value=value) | ||
| obj = self._object_mapper(unit, value, _LIST_OF_UNITS_BIT, self.bit) | ||
| if not obj: |
| if unit in ['yottabyte', 'YB', 'YiB']: | ||
| return self.yottabyte(value=value) | ||
| obj = self._object_mapper(unit, value, _LIST_OF_UNITS_BYTE, self.byte) | ||
| if not obj: |
Abstracted out the functionality and hence reducing number of "if's" in the code.