HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Custom checksum algorithm in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checksumcustompythonalgorithm

Problem

How can I make the following checksum algorithm more pythonic and readable?

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 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 checksum


We can see that the test you've mentioned in the question passes:

$ ipython3 -i test.py 
In [1]: get_checksum("foo", "one")
Out[1]: 9167

Code 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]: 9167

Context

StackExchange Code Review Q#154715, answer score: 8

Revisions (0)

No revisions yet.