10
\$\begingroup\$

I'm pretty very new to python, so I thought that I'd write a game to teach me python and better teach me how to think in hexadecimal and binary. I'm starting off by writing a few functions to convert between the different bases.

I'm looking for feedback on any better ways to implement these functions. There might be python libraries to do exactly this but I don't care. I can't think of a better way to check for the values A-F, and what they are in different bases, so if there are any better (prettier) ways to do it (I couldn't seem to find a switch/case in python, but maybe I didn't look hard enough).

As well as any possible better implementation, I'd also like input on naming and layout and whatnot.

Hex to binary function:

import re

def hexToBi(hexStr):
    hexRegex = "[A-Fa-f0-9]+"
    if not bool(re.match(hexRegex, hexStr)):
        return ""

    biStr = ""

    for c in hexStr:
        if c == '0':
            biStr += "0000 "
        elif c == '1':
            biStr += "0001 "
        elif c == '2':
            biStr += "0010 "
        elif c == '3':
            biStr += "0011 "
        elif c == '4':
            biStr += "0100 "
        elif c == '5':
            biStr += "0101 "
        elif c == '6':
            biStr += "0110 "
        elif c == '7':
            biStr += "0111 "
        elif c == '8':
            biStr += "1000 "
        elif c == '9':
            biStr += "1001 "
        elif c.upper() == 'A':
            biStr += "1010 "
        elif c.upper() == 'B':
            biStr += "1011 "
        elif c.upper() == 'C':
            biStr += "1100 "
        elif c.upper() == 'D':
            biStr += "1101 "
        elif c.upper() == 'E':
            biStr += "1110 "
        elif c.upper() == 'F':
            biStr += "1111 "

    return biStr

Binary to Hex function

def biToHex(biStr):
    biSpRegex = "^([01]{4}[\s^\s])+$"
    biRegex = "^([01]{4})+$"

    if not bool(re.match(biSpRegex, biStr)):
        biStr = biStr.replace(" ", "")
    if not bool(re.match(biRegex, biStr)):
        return ""

    nibbles = [biStr[i:i+4] for i in range(0, len(biStr), 4)]

    hexStr = ""

    for s in nibbles:
        val = 0
        if s[0] == '1':
            val = 8
        if s[1] == '1':
            val += 4
        if s[2] == '1':
            val += 2
        if s[3] == '1':
            val += 1

        if val < 10:
            hexStr += str(val)
        elif val == 10:
            hexStr += 'A'
        elif val == 11:
            hexStr += 'B'
        elif val == 12:
            hexStr += 'C'
        elif val == 13:
            hexStr += 'D'
        elif val == 14:
            hexStr += 'E'
        elif val == 15:
            hexStr += 'F'
    return hexStr

Binary and Hex to decimal functions

def hexToDec(hexStr):
    hexRegex = "[A-Fa-f0-9]+"
    if not bool(re.match(hexRegex, hexStr)):
        return ""

    val = 0;    
    power = len(hexStr) - 1
    for i in range(0, len(hexStr)):
        valAt = 0
        if hexStr[i].upper() == 'A':
            valAt = 10
        elif hexStr[i].upper() == 'B':
            valAt = 11
        elif hexStr[i].upper() == 'C':
            valAt = 12
        elif hexStr[i].upper() == 'D':
            valAt = 13
        elif hexStr[i].upper() == 'E':
            valAt = 14
        elif hexStr[i].upper() == 'F':
            valAt = 15
        elif int(hexStr[i]) < 10:
            valAt = int(hexStr[i])

        valAt = valAt * (16**power)
        power -= 1
        val += valAt

    return val

def biToDec(biStr):
    return hexToDec(biToHex(biStr))

very quick and limited set of unit tests

def unitTest(): 
    assert (hexToBi("ABC09") == "1010 1011 1100 0000 1001"), "Logic fail, \"1010 1011 1100 0000 1001\" expected, " + hexToBi("ABC09") + " recieved"
    assert (hexToBi("QWER") == ""), "Logic fail, \"\" expected, " + hexToBi("QWER") + " recieved"
    assert (biToHex("1111 1111 0000") == "FF0"), "Logic fail, \"FF0\" expected, " + biToHex("1111 1111 0000") + " recieved"
    assert (biToHex("1101 0000") == "D0"), "Logic fail, \"D0\" expected, " + biToHex("1101 0000") + " recieved"
    assert (hexToDec("FAC") == 4012), "Logic fail, \"4012\" expected, " + hexToDec("FAC")  + " recieved"
    assert (biToDec("1101 0111") == 215), "Logic fail, \"215\" expected, " + biToDec("1101 0111") == 215 + " recieved"
\$\endgroup\$
5
\$\begingroup\$

For the first function, I'd compare to None, as this is what match() documents returning when it fails (maybe a MatchObject always converts to True but I'm not sure).

Also in my mind it's better to fail than return a meaningless result (unless your function documents that it returns empty string in case of failure):

if re.match(hexRegex, hexStr) is None:
    raise Exception("some failure message")

Then I would use a map instead of the big if sequence:

conversions = {
    '0': "0000", # strange use of 2 different string delimiters here
    '1': "0001",
    ...
}

You can also use a more functional accumulation:

convert_digit = lambda d : conversions[d]
result_digit_groups = map(convert_digit, hexStr)

Or just:

result_digit_groups = [conversions[d] for d in hexStr]

Either way, finally concatenate with:

return " ".join(result_digit_groups) # note that you don't get an extra space at the end with join
\$\endgroup\$
  • 1
    \$\begingroup\$ "Documentation" is giving this project more credit than it deserves. What did you mean by strange use of 2 string delims? \$\endgroup\$ – Yann Oct 23 '14 at 13:45
  • \$\begingroup\$ By documentation I mean any kind of comment giving this precision to potential callers of the function. In python you can use "this" or 'that' for string literals. Unlike C++ for example, 'x' is not a character, it is the same as "x". Sometimes project/team-level coding conventions can tell you which one to prefer, and it's not very clean to mix both in the same code (unless there is a good reason, like the string containing some quotes). \$\endgroup\$ – Gnurfos Oct 23 '14 at 14:25
  • \$\begingroup\$ Ah, didn't realise that " and ' are the same, I'm coming to this from mostly C++ \$\endgroup\$ – Yann Oct 23 '14 at 14:27
  • \$\begingroup\$ if match: is fine to use. It even reads pretty well (if (there was a) match: do something). In python all objects are truthy by default. The only case when this is not true is when there is some well-known notion of truth for that kind of object that can be used. In this case there isn't. \$\endgroup\$ – Bakuriu Oct 23 '14 at 18:42
8
\$\begingroup\$

There is a shorter way to implement these converter functions, with the help of the int, bin and hex built-in Python functions:

def hex2bin(hex_str):
    # note: int('...', 16) parses from string as a hexadecimal number
    #       for example: int('deadbeef', 16) -> 3735928559
    # note: bin(int_num) formats `int_num` to binary string format
    #       for example: bin(5) -> '0b101'
    bin_str = bin(int(hex_str, 16))[2:]
    return ' '.join(bin_str[i:i+4] for i in xrange(0, len(bin_str), 4))
    # ^^^ only to pass your assertions. I would prefer this simpler version:
    #return bin(int(hex_str, 16))[2:]


def bin2hex(bin_str):
    # note: int('...', 2) parses from string as a hexadecimal number
    #       for example: int('101', 2) -> 5
    # note: hex(int_num) formats `int_num` to hexadecimal string format
    #       for example: hex(3735928559) -> '0xdeadbeaf'
    return hex(int(bin_str.replace(' ', ''), 2))[2:].upper()
    # ^^^ only to pass your assertions. I would prefer this simpler version:
    #return hex(int(bin_str, 2))[2:]


def from_hex(hex_str):
    return int(hex_str, 16)


def from_bin(bin_str):
    return from_hex(bin2hex(bin_str))


assert (hex2bin("ABC09") == "1010 1011 1100 0000 1001"), "Logic fail, \"1010 1011 1100 0000 1001\" expected, " + hex2bin("ABC09") + " recieved"
# assert (hex2bin("QWER") == ""), "Logic fail, \"\" expected, " + hex2bin("QWER") + " recieved"
assert (bin2hex("1111 1111 0000") == "FF0"), "Logic fail, \"FF0\" expected, " + bin2hex("1111 1111 0000") + " recieved"
assert (bin2hex("1101 0000") == "D0"), "Logic fail, \"D0\" expected, " + bin2hex("1101 0000") + " recieved"
assert (from_hex("FAC") == 4012), "Logic fail, \"4012\" expected, " + from_hex("FAC")  + " recieved"
assert (from_bin("1101 0111") == 215), "Logic fail, \"215\" expected, " + from_bin("1101 0111") == 215 + " recieved"

In addition to formatting according to PEP8, it's also recommended to use snake_case for function names and variable names.

If you notice I used 2 kinds of naming pattern:

  • hex2bin and bin2hex convert from string to string
    • Btw, using "bin" for binary instead of "bi" is also better in my opinion
  • from_hex and from_bin convert from string representations to integer

These are qualitatively different, that's why I separated them.

As I left in embedded comments, the implementations could be shorter and simpler if you could give up on some of your requirements. The good thing about the simpler versions is that Python's built-in tools can work with them directly, without additional transformations like removing spaces from 1111 1111 0000.

Note that I left one of your assertions broken on purpose and commented out. I think it's good to take a hint from the built-in tools and raise a ValueError if anyone tries to convert an invalid number. This kind of harsh response to invalid input is usually a good thing, and helps avoiding potentially nasty bugs.

Code review

On top of what others have already said, there are a couple of things that can be improved about your code worth pointing out.

def hexToBi(hexStr):
    hexRegex = "[A-Fa-f0-9]+"
    if not bool(re.match(hexRegex, hexStr)):
        return ""

Instead of compiling a regex every time the function is called, it's better to compile it once and reuse. Define the compiled version as a global constant, for example:

re_hex_format = re.compile(r"[A-Fa-f0-9]+")

And then in your function:

def hexToBi(hexStr):
    if not bool(re_hex_format.match(hexStr)):
        return ""

Also, use r"..." for strings that are regular expressions. For more details, see this page in the docs.

Finally, no need to wrap the output of .match(...) calls in a bool, this works just as well:

if not re_hex_format.match(hexStr):
    return ""

Since in case of no match, you get a None which is falsy, and in case of a match you get a match object which is truthy. If you read through PEP8, this treatment of "truth" is encouraged in Python.

About this:

re.match(r"^([01]{4})+$", biStr)):

Note that re.match applies the pattern at the beginning of the string, so the ^ is implied. The $ is indeed necessary. From help(re.match):

match(pattern, string, flags=0)
    Try to apply the pattern at the start of the string, returning
    a match object, or None if no match was found.
\$\endgroup\$
6
\$\begingroup\$

Style

In Python, there is a style guide called PEP8 : you don't have the obligation to comply to it but it is quite if we all follow the same conventions. You'll find various tools to check your code and/or to fix it : pep8, autopep8. You'll also find various other tools to check your code.

First thing I have spotted is the fact that the naming convention does not follow PEP8.

Don't repeat yourself

At the moment, you call upper() multiple times in your hexToBi function. You might as well just write : for c in hexStr.upper().

Data over code

Sometimes, storing pure data in the appropriated data structure will make your code much shorter, easier to read, easier to maintain and sometimes more efficient.

Here, you could define a dictionnary mapping hexa 'digits' to their binary conversion :

conv = {
    '0': "0000 ",
    '1': "0001 ",
    '2': "0010 ",
    '3': "0011 ",
    '4': "0100 ",
    '5': "0101 ",
    '6': "0110 ",
    '7': "0111 ",
    '8': "1000 ",
    '9': "1001 ",
    'A': "1010 ",
    'B': "1011 ",
    'C': "1100 ",
    'D': "1101 ",
    'E': "1110 ",
    'F': "1111 ",
}
biStr = ""

for c in hexStr.upper():
    biStr += conv[c]
return biStr

Even better, this can be written in a much better way taking into account comment from PEP8 :

For example, do not rely on CPython's efficient implementation of in-place string concatenation for statements in the form a += b or a = a + b. This optimization is fragile even in CPython (it only works for some types) and isn't present at all in implementations that don't use refcounting. In performance sensitive parts of the library, the ''.join() form should be used instead. This will ensure that concatenation occurs in linear time across various implementations.

And your code becomes :

return ''.join(conv[c] for c in hexStr.upper())

We can make this even better by fixing a small issue with your code : the trailing whitespace at the end : we can ask join to add a whitespace between chunks. Your code becomes :

def hexToBi(hexStr):
    conv = {
        '0': "0000",
        '1': "0001",
        '2': "0010",
        '3': "0011",
        '4': "0100",
        '5': "0101",
        '6': "0110",
        '7': "0111",
        '8': "1000",
        '9': "1001",
        'A': "1010",
        'B': "1011",
        'C': "1100",
        'D': "1101",
        'E': "1110",
        'F': "1111",
    }
    return ' '.join(conv[c] for c in hexStr.upper())

The dictionnary could be moved out of the function (and given a better name but I'll leave this for you).

A probably better way to write this

I think the best way to convert binary to hexa and hexa to binary is to write a more generic function converting binary/hexa/decimal representations to the actual number and actual number to their binary/hexa/decimal representations.

The signatures would be something like :

def convert_string_to_number(s, base=10)
def convert_number_to_string(n, base=10)

Of course, these could usually be defined using Python builtins but you might find this to be an interesting exercise.

\$\endgroup\$
  • \$\begingroup\$ Leaving out the dictionary idea, which I have implemented following @Gnurfos' advice, which has also gotten rid of the repetition of upper(), at a glance, am I doing anything that's particularly ugly (with pep8 in mind)? And I'm not sure what you mean by writing a more generic function, are you talking about having 1 function that takes the number (in any base) as well as a desired base to output as a string/whatever? Or something else? \$\endgroup\$ – Yann Oct 23 '14 at 14:12
  • \$\begingroup\$ I've edited my answer to try to add missing details. Please let me know if you still have any questions. \$\endgroup\$ – Josay Oct 23 '14 at 14:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.