Implementation of bitwise XOR function for bytes object

[adamantike]

This function is needed for future OAEP implementation, that makes use of bitwise XOR between bytes objects. Related to #68.

[coveralls]

Coverage increased (+0.05%) to 91.784% when pulling 28f89a0 on adamantike:xor-bytes into cf124d3 on sybrenstuvel:master.

[sybrenstuvel]

We're doing quite a bit of "try Python 2, and if it explodes, assume Python 3" in the code now. To be honest, I'm not too fond of it, especially in a function like this. What would you say about something like this in _compat.py?

import sys
PY3 = sys.version_info < (3, 0)

Then in the code (mostly in _compat.py) we can do longer chains of this:

if PY3:
    range = range
    integer_types = (int,)
    unicode_type = str
    def byte_literal(s):
        return s.encode('latin1')
else:
    range = xrange
    integer_types = (int, long)
    unicode_type = unicode
    def byte_literal(s):
        return s

I think this is more explicit and easier to read. It also performs better in your xor_bytes function, as raising & catching exceptions is much slower than if PY3. What do you think about this?

[sybrenstuvel]

By the way, now that we don't support Python 2.6 any more, we can drop the byte_literal function and just use the b'xxx' notation. I've added #74 for this.

[adamantike]

I think that new approach is a big design improvement! Even we could have boolean values for Python subversions in cases where a better implementation exists for newer subversions (i.e. PY3_4). Also, we don't make all code design in _compat.py based on explode things just to fallback to valid implementation.

However, I would like to keep the useful current just-one function implementation, to keep a separated and atomized logic, and also to not repeat docstrings. byte_literal isn't the best example, but for xor_bytes I think it could be defined this way:

if PY3:
    (...)
else:
    (...)

def xor_bytes(b1, b2, is_python3=PY3):
    """Docstring"""

    if is_python3:
        return ...
    else:
        return ...

Another alternative to hide is_python3 parameter overwrite is to make a common function:

def xor_bytes(b1, b2):
    return common_xor_bytes(b1, b2)

def common_xor_bytes(b1, b2, is_python3=PY3):
    """Docstring"""

    if is_python3:
        return ...
    else:
        return ...

What do you think about it?

Of course, I'll miss having a variable definition for py2/py3 on consecutive lines to compare them, but I can live with it.

[coveralls]

Coverage increased (+0.05%) to 91.841% when pulling c511bf1 on adamantike:xor-bytes into 505a25a on sybrenstuvel:master.

[adamantike]

Rebased, and removed all byte_literal usages on new tests.

[sybrenstuvel]

Test with some random (but still hard-coded) byte strings. It's too easy to miss subtle errors when you only test with all-on or all-off values.

[sybrenstuvel]

I think the first def xor_bytes(b1, b2, is_python3=PY3): looks good, better than the catching of the TypeError currently in the patch. This also makes Python 3 more of a "default", and Python 2 the "exceptional case", which I like.

[adamantike]

Great! I will create a new PR to refactor the compat file based on PY3, and we can come back to this one after it has been merged. Thanks @sybrenstuvel

[sybrenstuvel]

Please see my note on #82 (comment)

[adamantike]

@sybrenstuvel rebased with latest PY2 changes! I also added "random" tests

[coveralls]

Coverage decreased (-0.08%) to 91.686% when pulling ad80f80 on adamantike:xor-bytes into 81f0e95 on sybrenstuvel:master.

[sybrenstuvel]

Great work, thanks!