2
\$\begingroup\$

This is a Python (3) script that converts numbers among 5 data formats: binary, decimal, hexadecimal and octal, it supports 16 conversions, that is, it can convert each of the five data formats to and from the other four data formats.

This script is adapted from my previous PowerShell script that does similar things: PowerShell script binary-decimal-hexadecimal-ipv4 convertor

I know Python 3 has three builtin functions that can convert decimal numbers to respective formats: bin(), hex(), and oct(), and I know I can make a binary number decimal by adding 0b to it, and 0x for hexadecimal and 0o for octal, but this is about what I might come up with if I don't use builtin, and there isn't a builtin function to convert IPv4 to decimal and vice versa.

Anyway this script uses 16 switches to identify the source format and target format, the switches are two-letter strings, the first letter specifies the source format and the second letter specifies the target, the letters are the initials of the formats(binary, decimal, hexadecimal, IPv4 and octal), the 16 switches are:

bd, bh, bi, bo
db, dh, di, do
hb, hd, hi, ho
ob, od, oh, oi

Use it like this:

numconversion.py 0b11111101 bd

It can accept strings with and without the headers (0b, 0x and 0o), and can detect whether the input is valid or not, and throws errors if the input isn't valid.

So here is the code:

import re
import sys

NIBBLES = (
    ('0000', 0,  '0'),
    ('0001', 1,  '1'),
    ('0010', 2,  '2'),
    ('0011', 3,  '3'),
    ('0100', 4,  '4'),
    ('0101', 5,  '5'),
    ('0110', 6,  '6'),
    ('0111', 7,  '7'),
    ('1000', 8,  '8'),
    ('1001', 9,  '9'),
    ('1010', 10, 'a'),
    ('1011', 11, 'b'),
    ('1100', 12, 'c'),
    ('1101', 13, 'd'),
    ('1110', 14, 'e'),
    ('1111', 15, 'f')
)

TYPES = (
    ('binary number', '0b' + '1' * 32),
    ('decimal natural number', 4294967295),
    ('hexadecimal number', '0x' + 'f' * 8),
    ('IPv4 address', '255.255.255.255'),
    ('octal number', '0o' + '3' + '7' * 10)
)

ERR = (
    "InvalidData: The inputted string isn't a valid {0}, process will now stop.",
    "LimitsExceeded: The value of the inputted {0} exceeds the maximum IPv4 value possible, process will now stop.(maximum value allowed: {1})"
)

HEADERS = ('0b', '0d', '0x', 'ip', '0o')


def splitter(regex, inv):
    return list(filter(''.__ne__, re.split(regex, inv)))


def translator(inv, task):
    a, b = task
    outv = []
    for i in inv:
        for j in NIBBLES:
            if j[a] == i:
                outv.append(j[b])
    if b == 1:
        outv.reverse()
    return outv


def worker(inv, bits, task):
    outv = []
    for bit in bits:
        if inv >= bit:
            n = inv // bit
            inv = inv % bit
            if task in (0, 2):
                for i in NIBBLES:
                    if i[1] == n:
                        outv.append(i[task])
            elif task in (3, 4):
                outv.append(str(n))
        elif inv < bit and task in (0, 2):
            outv.append(NIBBLES[0][task])
        elif inv < bit and task in (3, 4):
            outv.append('0')
    return outv


def converter(inv, task, base):
    bits = []
    if task == 1:
        decv = 0
        for p, n in enumerate(inv):
            decv += n * base ** p
        return decv
    elif task in (0, 2, 4):
        i = 1
        while i <= inv:
            bits.append(i)
            i *= base
        bits.reverse()
        return HEADERS[task] + ''.join(worker(inv, bits, task)).lstrip('0')
    elif task == 3:
        for i in range(3, -1, -1):
            bits.append(base ** i)
        return '.'.join(worker(inv, bits, 3))


def binsub(binv, task):
    if re.match('^(0b)?[01]+$', binv):
        binv = binv.replace('0b', '')
        if task in (1, 2):
            if len(binv) % 4 > 0:
                binv = binv.zfill(4 * (len(binv) // 4 + 1))
            bits = translator(splitter('([01]{4})', binv), (0, task))
            if task == 1:
                return converter(bits, 1, 16)
            elif task == 2:
                return '0x' + ''.join(bits).lstrip('0')
        elif task == 4:
            bits = translator(splitter('([01]{4})', binv), (0, 1))
            return decsub(converter(bits, 1, 16), 4)
        elif task == 3:
            if re.match('^[01]{1,32}$', binv):
                return converter(binsub(binv, 1), 3, 256)
            else:
                print('\x1b[30;3;41m' + ERR[1].format(TYPES[0][0], TYPES[0][1]) + '\x1b[0m')
    else:
        print('\x1b[30;3;41m' + ERR[0].format(TYPES[0][0]) + '\x1b[0m')


def decsub(decv, task):
    if str(decv).isdigit():
        decv = int(decv)
        if task in (0, 2):
            return converter(decv, task, 16)
        elif task == 4:
            return converter(decv, 4, 8)
        elif task == 3:
            if decv <= 4294967295:
                return converter(decv, 3, 256)
            else:
                print('\x1b[30;3;41m' + ERR[1].format(TYPES[1][0], TYPES[1][1]) + '\x1b[0m')
    else:
        print('\x1b[30;3;41m' + ERR[0].format(TYPES[1][0]) + '\x1b[0m')


def hexsub(hexv, task):
    hexv = hexv.lower()
    if re.match('^(0x)?[0-9a-f]+$', hexv):
        hexv = hexv.replace('0x', '')
        if task in (0, 1):
            bits = translator(splitter('([0-9a-f])', hexv), (2, task))
            if task == 0:
                return '0b' + ''.join(bits).lstrip('0')
            elif task == 1:
                return converter(bits, 1, 16)
        elif task == 4:
            bits = translator(splitter('([0-9a-f])', hexv), (2, 1))
            return decsub(converter(bits, 1, 16), 4)
        elif task == 3:
            if re.match('^[0-9a-f]{1,8}$', hexv):
                return converter(hexsub(hexv, 1), 3, 256)
            else:
                print('\x1b[30;3;41m' + ERR[1].format(TYPES[2][0], TYPES[2][1]) + '\x1b[0m')
    else:
        print('\x1b[30;3;41m' + ERR[0].format(TYPES[2][0]) + '\x1b[0m')


def ip4sub(ipv, task):
    if re.match('^((25[0-5]|2[0-4]\d|[01]?\d\d?)\.){3}(25[0-5]|2[0-4]\d|[01]?\d\d?)$', ipv):
        segs = list(map(int, ipv.split('.')))
        segs.reverse()
        decv = converter(segs, 1, 256)
        if task == 1:
            return decv
        elif task in (0, 2, 4):
            return decsub(decv, task)
    else:
        print('\x1b[30;3;41m' + ERR[0].format(TYPES[3][0]) + '\x1b[0m')


def octsub(octv, task):
    if re.match('^(0o)?[0-7]+$', octv):
        octv = octv.replace('0o', '')
        bits = list(map(int, splitter('([0-7])', octv)))
        bits.reverse()
        decv = converter(bits, 1, 8)
        if task == 1:
            return decv
        elif task in (0, 2):
            return decsub(decv, task)
        elif task == 3:
            if decv <= 4294967295:
                return decsub(decv, 3)
            else:
                print('\x1b[30;3;41m' + ERR[1].format(TYPES[4][0], TYPES[4][1]) + '\x1b[0m')
    else:
        print('\x1b[30;3;41m' + ERR[0].format(TYPES[4][0]) + '\x1b[0m')


def main(inv, task):
    a, b = task
    tasks = {'b': 0, 'd': 1, 'h': 2, 'i': 3, 'o': 4}
    if {a, b}.issubset(tasks.keys()) and a != b:
        if a == 'b':
            print(binsub(inv, tasks[b]))
        elif a == 'd':
            print(decsub(inv, tasks[b]))
        elif a == 'h':
            print(hexsub(inv, tasks[b]))
        elif a == 'i':
            print(ip4sub(inv, tasks[b]))
        elif a == 'o':
            print(octsub(inv, tasks[b]))
    else:
        print('\x1b[30;3;41m' + 'InvalidOperation: The operation specified is undefined, process will now stop.' + '\x1b[0m')


if __name__ == '__main__':
    inv, task = sys.argv[1:]
    main(inv, task)

The functions are reused heavily, I have made the code as simple as possible, and eliminated duplication as much as I can, though I am sure it can be simpler.

Please help me simplify my code and further eliminate code duplication and make it as simple as possible while maintaining the same logic, thank you.

\$\endgroup\$
3
\$\begingroup\$

I have made the code as simple as possible

No you haven't, because you've arbitrarily decided to avoid a subset of built-ins. Any simplification advice I would give would be predicated on proper use of the standard library which is not the case here. Other points:

  • Your NIBBLES and BYTES would be a good fit for namedtuple
  • NIBBLES second element is redundant and can be assumed to be the index in the tuple
  • Represent 4294967295 as 0xFFFF_FFFF
  • list(filter(''.__ne__, re.split(regex, inv))) is clearer as [group for group in re.split(regex, inv) if group != '']
  • translator can drop outv and replace append with yield, so long as you externalize the reversal. a and b are very poor variable names, and task seems like it should be two separate arguments rather than a packed string.
  • Do not unconditionally bake in ANSI escape sequences. You're not guaranteed a real TTY, so this will explode in dramatic fashion if (for example) you attempt to | less.
  • Given how impenetrable some of your variable names are, at least type hinting them will help reveal some of the mystery.

A bare-bones implementation that is not strictly equivalent - but closer to properly using the standard library - is, for example,

#!/usr/bin/env python3

from dataclasses import dataclass
from functools import partial
from ipaddress import IPv4Address
from sys import argv
from typing import Callable


@dataclass
class Base:
    parse: Callable[[str], int]
    format: Callable[[int], str]


BASES = {
    'b': Base(partial(int, base=2), '{:_b}'.format),
    'o': Base(partial(int, base=8), '{:_o}'.format),
    'd': Base(int, str),
    'h': Base(partial(int, base=16), '{:_X}'.format),
    'i': Base(
        lambda s: int(IPv4Address(s)),
        lambda i: str(IPv4Address(i)),
    ),
}


def convert(original: str, task: str) -> str:
    source_letter, dest_letter = task
    source, dest = BASES.get(source_letter), BASES.get(dest_letter)

    if source is None:
        raise ValueError(f'"{source_letter}" is not a valid base')
    if dest is None:
        raise ValueError(f'"{dest_letter}" is not a valid base')

    value = source.parse(original)
    return dest.format(value)


if __name__ == '__main__':
    print(convert(*argv[1:]))
\$\endgroup\$
2
\$\begingroup\$

General comment

Your code looks good to me from an coding-style point of view and is split into various small functions which is great.

A pretty easy way to make it much better would be to add documentation:

  • Comments for the different constants
  • Docstrings for module and functions

Let's see how various other details can be improved (in no specific order because I have yet to understand all the details in the code).

Main interface

You've defined a main function and a if __name__ == '__main__' guard which is a great habit.

Here are a few things which could make things even better:

  • Argument handling

    • Check the number of arguments provided before using sys.argv[1:] to provide a clear error message to the user when the number of argument is incorrect
    • Similarly, by performing a, b = task, you assume that task is (a string) of length 2 which may not be the case
  • Signature for the main function

    • The name inv stills looks weird to me: I have no idea what it means in that particular context. My suggestions would be things about numbers and strings such as: num_string, str_num, s, n or maybe input_str
    • The name task is a bit clearer but my suggestions would be something about formats, conversions or bases such as formatting_rule.
    • The task parameter is not really interesting as such: the first thing we do is decompose its 2 parts into a and b. The function would be easier to understand if it took 3 parameters which could be named: input_str, input_format, output_format.

At this stage, we have something like:

def main(input_str, input_format, output_format):
    formats = {
        'b': 0,
        'd': 1,
        'h': 2,
        'i': 3,
        'o': 4
    }
    if {input_format, output_format}.issubset(formats.keys()) and input_format != output_format:
        if input_format == 'b':
            print(binsub(input_str, formats[output_format]))
        elif input_format == 'd':
            print(decsub(input_str, formats[output_format]))
        elif input_format == 'h':
            print(hexsub(input_str, formats[output_format]))
        elif input_format == 'i':
            print(ip4sub(input_str, formats[output_format]))
        elif input_format == 'o':
            print(octsub(input_str, formats[output_format]))
    else:
        print('\x1b[30;3;41m' + 'InvalidOperation: The operation specified is undefined, process will now stop.' + '\x1b[0m')


if __name__ == '__main__':
    # TODO: Better parameter handling
    input_str, (input_format, output_format) = sys.argv[1:]
    main(input_str, input_format, output_format)

More details about the main function

The validity of the output/input format is pretty smart, relying on set, subset, dictionnary keys... Unfortunately, it is one of those place where simpler is probably better. Indeed, you could just check one parameter at a time which would also give a way to have a more precise error message ("THAT particular parameter has in unexpected value").

We would get something like:

def main(input_str, input_format, output_format):
    formats = {
        'b': 0,
        'd': 1,
        'h': 2,
        'i': 3,
        'o': 4
    }
    if input_format not in formats:
        print('\x1b[30;3;41m' + 'InvalidOperation: The input format ("%s") is invalid, process will now stop.' % input_format + '\x1b[0m')
    elif output_format not in formats:
        print('\x1b[30;3;41m' + 'InvalidOperation: The output format ("%s") is invalid, process will now stop.' % output_format + '\x1b[0m')
    elif input_format == output_format:
        print('\x1b[30;3;41m' + 'InvalidOperation: The input/output format are identical ("%s") is invalid, process will now stop.' % output_format + '\x1b[0m')
    elif input_format == 'b':
        print(binsub(input_str, formats[output_format]))
    elif input_format == 'd':
        print(decsub(input_str, formats[output_format]))
    elif input_format == 'h':
        print(hexsub(input_str, formats[output_format]))
    elif input_format == 'i':
        print(ip4sub(input_str, formats[output_format]))
    elif input_format == 'o':
        print(octsub(input_str, formats[output_format]))
    else:
        assert False # This should not happen

A first thing that I'd like to point out: converting a number from an input format to an identical output format should probably be considered a valid operation: it would make the program/function easier to use but also, it would make it much easier to test. If converting "42" from decimal to decimal gives me anything else, this will be very straigthforward to notice. At the moment, your implementation does not (easily) easily support this but this is something that will be tackled later.

Finally, because we are already relying on a dictionnary, we could try to fit all the data we need into that data structure to get rid of any superfluous code.

Things would look like:

def main(input_str, input_format, output_format):
    formats = {
        'b': (0, binsub),
        'd': (1, decsub),
        'h': (2, hexsub),
        'i': (3, ip4sub),
        'o': (4, octsub),
    }
    if input_format not in formats:
        print('\x1b[30;3;41m' + 'InvalidOperation: The input format ("%s") is invalid, process will now stop.' % input_format + '\x1b[0m')
        return
    _, input_func = formats[input_format]
    if output_format not in formats:
        print('\x1b[30;3;41m' + 'InvalidOperation: The output format ("%s") is invalid, process will now stop.' % output_format + '\x1b[0m')
        return
    output_code, _ = formats[output_format]
    if input_format == output_format:  # TODO: Remove this eventually
        print('\x1b[30;3;41m' + 'InvalidOperation: The input/output format are identical ("%s") is invalid, process will now stop.' % output_format + '\x1b[0m')
    print(input_func(input_str, output_code))

Error formatting

You are using the keycode for colors in the terminal which is a nice touch (and ensuring that the style goes back to normal afterward is great). Unfortunately, having the hard coded values everywhere in the code make it bloated and hard to understand.

Defining 2 simple constants makes things much clearer already.

# STYLES FOR TERMINAL
ITALIC_RED = '\x1b[30;3;41m'
NORMAL = '\x1b[0m'
...
print(ITALIC_RED + 'InvalidOperation: The input format ("%s") is invalid, process will now stop.' % input_format + NORMAL)

An even better approach could be to define a function for this so that if you want to change how things are displayed, you have a single place to update.

You'd get something like:

def show_error(msg):
    """Show error with message msg."""
    print(ITALIC_RED + msg + NORMAL)

(Going further, one could also decide to print in stderr instead of stdout).

Error messages

The error messages are in a tuple and accessed by index when needed. It would be much clearer to define 2 constants for this:

We'd have:

# Error message for common conversion errors
INVALID_DATA_ERR = "InvalidData: The inputted string isn't a valid {0}, process will now stop."
LIMITS_EXCEEDED_ERR = "LimitsExceeded: The value of the inputted {0} exceeds the maximum IPv4 value possible, process will now stop.(maximum value allowed: {1})"

...

            else:
                show_error(LIMITS_EXCEEDED_ERR.format(TYPES[0][0], TYPES[0][1]))
    else:
        show_error(INVALID_DATA_ERR.format(TYPES[0][0]))

The same logic applies to the values you put into the error messages. The TYPES tuple is accessed by index, it would be easier to define various constants:

# Description (name, limit) of the different formats to be used in error messages
BIN_DESCR = ('binary number', '0b' + '1' * 32)
DEC_DESCR = ('decimal natural number', 4294967295)
HEX_DESCR = ('hexadecimal number', '0x' + 'f' * 8)
IP4_DESCR = ('IPv4 address', '255.255.255.255')
OCT_DESCR = ('octal number', '0o' + '3' + '7' * 10)

...

            else:
                show_error(LIMITS_EXCEEDED_ERR.format(*HEX_DESCR))
    else:
        show_error(INVALID_DATA_ERR.format(*HEX_DESCR))

Conversion

I have not tackled the parts about the conversion yet but it looks like this got comments by other reviewers already.

My main concern is the fact that we always seem to intermix the logic about the input format and the logic about the output format (but I may be wrong entirely).

In my opinion, the different steps should be:

  • Convert the input string into a Python integer based on the input format (and maybe check the string validity)

  • Convert that input into a string based on the output format

This is a much better separation of the concerns. Also, adding a new supported format is much easier: you just need to be able to convert a number in a string and a string in a number without worrying about the already existing supporting formats.


Here is my take on this programming problem:



from collections import namedtuple

Format = namedtuple('Format', ['name', 'symbols', 'prefixes'])

def convert_string_to_number(string, input_fmt):
    """Convert a string to a number based on an input format."""
    # Create mapping from symbols to values
    symbols_values = { c: i for i, c in enumerate(input_fmt.symbols) }
    base = len(symbols_values)
    # Remove optional prefix (such as Ox)
    for prefix in input_fmt.prefixes:
        if string.startswith(prefix):
            string = string[len(prefix):]
            break
    # Perform the conversion
    n = 0
    for c in string:
        val = symbols_values.get(c, None)
        if val is None:
            show_error('Invalid symbol %s during conversion of %s in %s' % (c, string, input_fmt.name))
            return None
        n = n * base + val
    return n

def format_number(number, output_fmt):
    """Format a number based on an ouput format."""
    base = len(output_fmt.symbols)
    symbols = []
    while number:
        number, rem = divmod(number, base)
        symbols.append(output_fmt.symbols[rem])
    return "".join(reversed(symbols))


def main(input_str, input_format, output_format):
    formats = {
        'b': Format('binary', '01', []),
        'd': Format('decimal', '0123456789', []),
        'o': Format('octal', '01234567', []),
        'h': Format('hexadecimal', '0123456789ABCDEF', ['0x']),
    }
    input_fmt = formats.get(input_format, None)
    if input_fmt is None:
        show_error('InvalidOperation: The input format ("%s") is invalid, process will now stop.' % input_format)
        return
    output_fmt = formats.get(output_format, None)
    if output_fmt is None:
        show_error('InvalidOperation: The output format ("%s") is invalid, process will now stop.' % output_format)
        return
    n = convert_string_to_number(input_str, input_fmt)
    print(format_number(n, output_fmt))


if __name__ == '__main__':
    # TODO: Better parameter handling
    input_str, (input_format, output_format) = sys.argv[1:]

Various features are missing compared to your implementation but I think it gives a few ideas.

\$\endgroup\$
0
\$\begingroup\$

Hmm, after some hard working I have made huge improvements to my code, I have significantly reduced the number of lines it spans and rewritten almost everything.

It now uses bit shifts to handle the conversions, and it uses one line to convert the other four formats into decimal, and another line to convert decimal into the other four formats, with some conditional lines to help the conversions.

I now use one function to handle the data processing, and the script now checks arguments before running the function.

There might be further improvements but I have done absolutely my best for now, anyway here is the code:

import re
import sys
from collections import namedtuple

kind = namedtuple('kind', 'name header max regex slicer')

SWITCH = {'b': 0, 'd': 1, 'h': 2, 'i': 3, 'o': 4}

HEXD = (
    '0', '1', '2', '3',
    '4', '5', '6', '7',
    '8', '9', 'a', 'b',
    'c', 'd', 'e', 'f'
)

TYPES = (
    kind('binary number', '0b', '0b'+'1'*32, '^(0b)?[01]+$', '([01])'),
    kind('decimal natural', None, 4294967295, '^\d+$', None),
    kind('hexadecimal number', '0x', '0x'+'f' * 8, '^(0x)?[\da-f]+$', '([\da-f])'),
    kind('IPv4 address', None, '255.255.255.255', '^((25[0-5]|2[0-4]\d|[01]?\d\d?)\.){3}(25[0-5]|2[0-4]\d|[01]?\d\d?)$', '\.'),
    kind('octal number', '0o', '0o'+'3'+'7'*10, '^(0o)?[0-7]+$', '([0-7])')
)

ERR = (
    "InvalidData: The inputted string isn't a valid {0}, process will now stop.",
    "LimitsExceeded: The value of the inputted {0} exceeds the maximum IPv4 value possible, process will now stop.(maximum value allowed: {1})",
    'InvalidOperation: The operation specified is undefined, process will now stop.',
    "Invalid Operation: The number of arguments passed to the script isn't three, process will now stop"
)


def splitter(regex, val):
    return list(filter(''.__ne__, re.split(regex, val)))


def showerror(msg):
    return '\x1b[30;3;41m' + msg + '\x1b[0m'


def weights(val, base):
    expos = []
    a, b = 1, 0
    while a <= val:
        expos.append(b)
        a *= base
        b += 1
    expos.reverse()
    return expos


def converter(val, a, b):
    if b == 1:
        val.reverse()
        k = (1, None, 4, 8, 3)
        if a in (0, 3, 4):
            val = list(map(int, val))
        elif a == 2:
            val = [HEXD.index(i) for i in val]
        return sum([int(n) << k[a] * p for p, n in enumerate(val)])
    elif a == 1:
        c = ((2, 1), None, (16, 4), (256, 8), (8, 3))
        d, e = c[b]
        bits = []
        if b in (0, 2, 4):
            digits = weights(val, d)
        elif b == 3:
            digits = (3, 2, 1, 0)
        for f in digits:
            bit = (val >> e * f) % d
            if b in (0, 3, 4):
                bits.append(str(bit))
            elif b == 2:
                bits.append(HEXD[bit])
        if b in (0, 2, 4):
            return TYPES[b].header + ''.join(bits)
        elif b == 3:
            return '.'.join(bits)


def main(val, a, b):
    if re.match(TYPES[a].regex, val):
        if b != 3:
            if a == 1:
                val = int(val)
                return converter(val, 1, b)
            else:
                bits = splitter(TYPES[a].slicer, val.lstrip(TYPES[a].header))
                decv = converter(bits, a, 1)
                if b == 1:
                    return decv
                else:
                    return converter(decv, 1, b)
        else:
            if a == 1:
                decv = int(val)
            else:
                decv = main(val, a, 1)
            if decv <= 4294967295:
                return converter(decv, 1, 3)
            else:
                return showerror(ERR[1].format(TYPES[a].name, TYPES[a].max))
    else:
        return showerror(ERR[0].format(TYPES[a].name))


if __name__ == '__main__':
    if len(sys.argv[1:]) == 3:
        val, x, y = sys.argv[1:]
        if {x, y}.issubset(SWITCH.keys()) and x != y:
            x = SWITCH[x]
            y = SWITCH[y]
            print(main(val, x, y))
        else:
            print(showerror(ERR[2]))
    else:
        print(showerror(ERR[3]))
\$\endgroup\$

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.