patternpythonMinor
Custom checksum algorithm in Python
Viewed 0 times
checksumcustompythonalgorithm
Problem
How can I make the following checksum algorithm more pythonic and readable?
The thing called
This code as-is must return this:
items = {1:'one', 2:'two', 3:'three'}
text = "foo" # This variable may change on runtime. Added to allow the execution
number = 1 # This variable may change on runtime. Added to allow the execution
string = text + items[number] + " " + "trailing"
if items[number] == "two": string=text
string = [ord(x) for x in string]
iterator = zip(string[1:], string)
checksum = string[0]
for counter, (x, y) in enumerate(iterator):
if counter % 2:
checksum += x
checksum += 2 * y
else:
checksum += x * y
checksum %= 0x10000
print("Checksum:", checksum)The thing called
salt by everyone here is fixed and won't change ever. Also, the code is into a bigger function.This code as-is must return this:
Checksum: 9167
Solution
First of all, I would apply the "Extract Method" refactoring method and put the logic behind generating a checksum into a
Also, I would avoid hardcoding the
You can use
Comments and docstrings would definitely be needed to describe the logic behind the checksum calculations.
Variable naming can be improved -
At the end you would have something like:
We can see that the test you've mentioned in the question passes:
get_checksum() function.Also, I would avoid hardcoding the
trailing "salt" and the 0x10000 "normalizer" and put them into the default keyword argument value and to a "constant" respectively.You can use
sum() instead of having loop with if and else, which increase the overall nestedness. But, it might become less explicit and potentially a bit more difficult to understand.Comments and docstrings would definitely be needed to describe the logic behind the checksum calculations.
Variable naming can be improved -
str_a, str_b and string are not the best choices (even though the part1 and part2 in the code below are not either - think about what these strings represent - may be you'll come up with better variable names). Also, note that you are reusing the string variable to keep the "ords" of each character, which, strictly speaking, is not good - the variable name does not correspond to what it is used for. At the end you would have something like:
DEFAULT_SALT = "trailing"
NORMALIZER = 0x10000
def get_checksum(part1, part2, salt=DEFAULT_SALT):
"""Returns a checksum of two strings."""
combined_string = part1 + part2 + " " + salt if part2 != "***" else part1
ords = [ord(x) for x in combined_string]
checksum = ords[0] # initial value
# TODO: document the logic behind the checksum calculations
iterator = zip(ords[1:], ords)
checksum += sum(x + 2 * y if counter % 2 else x * y
for counter, (x, y) in enumerate(iterator))
checksum %= NORMALIZER
return checksumWe can see that the test you've mentioned in the question passes:
$ ipython3 -i test.py
In [1]: get_checksum("foo", "one")
Out[1]: 9167Code Snippets
DEFAULT_SALT = "trailing"
NORMALIZER = 0x10000
def get_checksum(part1, part2, salt=DEFAULT_SALT):
"""Returns a checksum of two strings."""
combined_string = part1 + part2 + " " + salt if part2 != "***" else part1
ords = [ord(x) for x in combined_string]
checksum = ords[0] # initial value
# TODO: document the logic behind the checksum calculations
iterator = zip(ords[1:], ords)
checksum += sum(x + 2 * y if counter % 2 else x * y
for counter, (x, y) in enumerate(iterator))
checksum %= NORMALIZER
return checksum$ ipython3 -i test.py
In [1]: get_checksum("foo", "one")
Out[1]: 9167Context
StackExchange Code Review Q#154715, answer score: 8
Revisions (0)
No revisions yet.