Create PY2 constant to simplify compatibility decisions #82
Conversation
| @@ -26,6 +26,8 @@ | |||
| MAX_INT32 = (1 << 31) - 1 | |||
| MAX_INT16 = (1 << 15) - 1 | |||
|
|
|||
| PY3 = sys.version_info[0] >= 3 | |||
There was a problem hiding this comment.
Let's make this == 3, as PY3 being True on Python 4.x would be rather silly.
There was a problem hiding this comment.
Yes, that would be more accurate. But as I see it, if Python 4 is released tomorrow, I would prefer to have py3's configurations rather than py2's (unless until a new PY4 constant is added).
With that in mind, what do you think about using a PY2 constant instead?
| return out.getvalue() | ||
| else: | ||
| def make_buffer(): | ||
| def make_buffer(): |
There was a problem hiding this comment.
Hmmmm now that I see how it turns out, I have some doubts about doing the if PY3 inside the function. The way it is now, if PY3 is evaluated for each and every function call. By doing something like this:
if PY3:
def write_to_stdout(data): ...
def make_buffer(): ...
else:
def write_to_stdout(data): ...
def make_buffer(): ...we only need to evaluate if PY3 once. This is slightly harder to read, as the PY3 and PY2 versions of the functions are potentially further apart, it does make the runtime execution a bit faster. What do you think @adamantike ?
There was a problem hiding this comment.
I try to prioritize readability and DRYness instead of (over)optimization. Even if these functions don't share logic, I prefer to have a unique point of "failure", one docstring to maintain and shared common blocks inside the function if that's possible.
In my opinion, those advantages will make devs to save a lot more time when trying to contribute to the project or follow a stack trace than one more comparison per call will ever provide. Does that make any sense?
There was a problem hiding this comment.
It makes sense, and I agree that readability is very important. However, I'm also curious as to the runtime performance impact. Do you have time to do some timing tests with some large key generation and file encryption?
adamantike
changed the title
|
@sybrenstuvel is there something else we should address for this PR? Also, should we consider using |
Based on discussion at #72, remove
try/except-based decisions and use the Python version to clean compatibility decisions