Your code is quite literally WET.
I'd recommend that you merge integer2bits
and integer2bytes
into one private function.
And for you to merge bits2integer
and bytes2integer
.
This allows for you to reduce the amount of testing needed.
As you now only need to test one function rather than two.
If you pass to them lists of numbers rather than bytes or strings than you can simplify the internal logic too.
Starting with integer2bits
and integer2bytes
to make _integer_convert
.
You should be able to see that the first five lines are nearly identical.
The only difference is how much you shift the number by.
And so if we pass a variable size
to the function to define the size of the data.
You can change the shift to 1 << width * size
.
def _integer_convert(number, width, size):
if width < 1:
raise ValueError("width must be positive")
number = int(number)
if number < 0:
number += 1 << width * size
After this you should notice you're using a bit mask when passing number
to int2byte
.
This mask is equal to (1 << size) - 1
.
You're also going through the number in chunks, where a chunk starts at i * size
.
And has the size of mask
.
This means that we can change the function to be:
def _integer_convert(number, width, size):
if width < 1:
raise ValueError("width must be positive")
number = int(number)
if number < 0:
number += 1 << width * size
mask = (1 << size) - 1
acc = [b"\x00"] * width
i = width - 1
while number > 0:
acc[i] = number & mask
number >>= size
i -= 1
return acc
This is good and all, but you can change it to a generator comprehension.
This is almost definitely slower than the current implementation.
But can be easier to read, if you're familiar with comprehensions.
The range
that we'll be using will start at width-1
, and end at 0
.
And so we can use range(width-1, -1, -1)
.
After this I'd make the chunks start, and then perform the mask on the data.
This allows for smaller code, through the use of comprehensions,
but is likely to be slower than the above function.
And terrible compared to the above with _integer_convert(0, 20, 1)
.
def _integer_convert(number, width, size):
if width < 1:
raise ValueError("width must be positive")
number = int(number)
if number < 0:
number += 1 << width * size
mask = (1 << size) - 1
chunks = (i * size for i in range(width-1, -1, -1))
return (((number & mask << chunk) >> chunk) for chunk in chunks)
To further improve this function I'd add doctests to it.
Your current doctests are limited and are actually erroneous.
And so using tuples as input and converting the iterator to a tuple as output can allow safer and more clear tests.
I'd also test all aspects of the function.
Some different sizes, and at least the sizes 1 and 8.
Some different numbers, and a couple of different widths.
But you almost definitely have to test a negative number.
You don't do this at the moment.
And I actually didn't change number += 1 << width
to number += 1 << width * size
originally and the new tests picked that up.
And so these tests are already better than your old tests.
Adding all these left me with the function:
def _integer_convert(number, width, size):
"""
>>> tuple(_integer_convert(170, 8, 1))
(1, 0, 1, 0, 1, 0, 1, 0)
>>> tuple(_integer_convert(170, 4, 2))
(2, 2, 2, 2)
>>> tuple(_integer_convert(170, 4, 3))
(0, 2, 5, 2)
>>> tuple(_integer_convert(170, 4, 4))
(0, 0, 10, 10)
>>> tuple(_integer_convert(170, 4, 5))
(0, 0, 5, 10)
>>> tuple(_integer_convert(170, 4, 6))
(0, 0, 2, 42)
>>> tuple(_integer_convert(170, 4, 7))
(0, 0, 1, 42)
>>> tuple(_integer_convert(170, 2, 8))
(0, 170)
>>> tuple(_integer_convert(170, 1, 8))
(170,)
>>> _integer_convert(170, 0, 8)
Traceback (most recent call last):
...
ValueError: width must be positive
>>> tuple(_integer_convert(-42, 8, 1))
(1, 1, 0, 1, 0, 1, 1, 0)
>>> tuple(_integer_convert(-170, 1, 8))
(86,)
>>> tuple(_integer_convert(-170, 2, 8))
(255, 86)
>>> tuple(_integer_convert(19, 8, 1))
(0, 0, 0, 1, 0, 0, 1, 1)
>>> tuple(_integer_convert(19, 2, 8))
(0, 19)
"""
if width < 1:
raise ValueError("width must be positive")
number = int(number)
if number < 0:
number += 1 << width * size
mask = (1 << size) - 1
chunks = (i * size for i in range(width-1, -1, -1))
return (((number & mask << chunk) >> chunk) for chunk in chunks)
After this we should redefine integer2bits
and integer2bytes
, these are going to be rather WET,
but writing everything twice rather than eight times is the lesser of two evils.
I'd also change your doctests to work in both Python 2 and Python 3.
But you can skimp on them here as you're only testing the joining of the strings/bytes.
And so can become:
def integer2bits(number, width):
r"""
Converts an integer into its binary representation in a b-string. Width is the amount of bits to generate. If width is larger than the actual amount of bits required to represent number in binary, sign-extension is used. If it's smaller, the representation is trimmed to width bits. Each bit is represented as either b'\x00' or b'\x01'. The most significant is first, big-endian. This is reverse to `bits2integer`.
Examples:
>>> str(integer2bits(19, 8).decode('utf-8'))
'\x00\x00\x00\x01\x00\x00\x01\x01'
"""
return b"".join(int2byte(i) for i in _integer_convert(number, width, 1))
def integer2bytes(number, width):
r"""
Converts a b-string into an integer. This is reverse to `bytes2integer`.
Examples:
>>> str(integer2bytes(19, 4).decode('utf-8'))
'\x00\x00\x00\x13'
"""
return b"".join(int2byte(i) for i in _integer_convert(number, width, 8))
After this we should change bits2integer
and bytes2integer
in the same way.
First I'd like to point out that (1 << (n - 1))*2 == 1 << n
.
The only difference is the latter doesn't error when n
is 0
.
And so you can simplify that line.
Another difference is number = (number << 1) | onebit2integer(b)
, instead I'd move onebit2integer
out of this function.
And so I'd change the for loop to iterate through iterateints(data)
and change this line to (number << size) | b & mask
.
This allows for any size
, and the special matches can be moved out of the function.
Finally if we pass iterateints(data)
as the data, we can simplify the check if the number is negative.
This is as we can get the first number, or default to zero, next(iter(data), 0)
.
And check if the correct bit is set, next(iter(data), 0) & 1 << (size - 1)
.
And finally adding the inverted function doctests from _integer_convert
, and some doctests for signed
,
we can get:
def _byte_convert(data, signed, size):
r"""
>>> _byte_convert((1, 0, 1, 0, 1, 0, 1, 0), False, 1)
170
>>> _byte_convert((2, 2, 2, 2), False, 2)
170
>>> _byte_convert((0, 2, 5, 2), False, 3)
170
>>> _byte_convert((0, 0, 10, 10), False, 4)
170
>>> _byte_convert((0, 0, 5, 10), False, 5)
170
>>> _byte_convert((0, 0, 2, 42), False, 6)
170
>>> _byte_convert((0, 0, 1, 42), False, 7)
170
>>> _byte_convert((0, 0, 0, 170), False, 8)
170
>>> _byte_convert((170,), False, 8)
170
>>> _byte_convert((1, 1, 0, 1, 0, 1, 1, 0), True, 1)
-42
>>> _byte_convert((1, 1, 0, 1, 0, 1, 1, 0), False, 1)
214
>>> _byte_convert((0, 1, 1, 0, 1, 0, 1, 1, 0), True, 1)
214
>>> _byte_convert((0, 1, 1, 0, 1, 0, 1, 1, 0), False, 1)
214
>>> _byte_convert((86,), True, 8)
86
>>> _byte_convert((255, 86), True, 8)
-170
>>> _byte_convert((1, 0, 1, 0, 1, 0, 1, 0), True, 1)
-86
>>> _byte_convert((2, 2, 2, 2), True, 2)
-86
>>> _byte_convert((170,), True, 8)
-86
"""
mask = (1 << size) - 1
number = 0
for num in data:
number = (number << size) | num & mask
if signed and next(iter(data), 0) & 1 << (size - 1):
return number - (1 << len(data) * size)
else:
return number
I said that we can move the onebit2integer
check out of the function for bits2integer
, and we can.
This is simply by passing [onebit2integer(i) for i in iteratebytes(data)]
rather than list(iterateints(data))
.
And now both functions function the same way.
So the functions bits2integer
and bytes2integer
can become:
def bits2integer(data, signed=False):
r"""
Converts a b-string into an integer. Both b'0' and b'\x00' are considered zero, and both b'1' and b'\x01' are considered one. Set sign to interpret the number as a 2-s complement signed integer. This is reverse to `integer2bits`.
Examples:
>>> bits2integer(b"\x01\x00\x00\x01\x01")
19
>>> bits2integer(b"10011")
19
"""
return _byte_convert([onebit2integer(i) for i in iteratebytes(data)], signed, 1)
def bytes2integer(data, signed=False):
r"""
Converts a b-string into an integer. This is reverse to `integer2bytes`.
Examples:
>>> bytes2integer(b'\x00\x00\x00\x13')
19
"""
return _byte_convert(list(iterateints(data)), signed, 8)
I've not really read the other functions, but they look ok.
I'd like to point out that Python's default style guide, PEP8,
says that we should use snake_case
for function names, and yours are not.
Style guides are mostly there to create consistency, which you have and so you may want to ignore this.
One way to change this could be to have two functions, one that is integer2bits
which calls integer_to_bits
.
And have integer2bits
warn on usage, after a couple of years you could, somewhat safely, remove the integer2bits
function.
This is as I use PEP8, and your functions look strange in my code.
My code is also unlikely to be faster than yours, as I didn't aim for speed.
This is as you were duplicating code, and I'd remove that before optimizing your code.