3
\$\begingroup\$

I debated long and hard before posting this question, and I did a lot of experimenting. I just can't seem to work out an 'elegant', concise way to get done what I want done in the manner I want it done. I found it very hard to research because of the difficulty in not knowing exactly what to search for.

Again, I am writing a converter for binary, decimal and hex using Tkinter and not using any of Python's built-in math functions.

In the code I have one class, within it many methods. I have the methods to convert from binary to decimal and hex working correctly (bin_to_dec & bin_to_hex, respectively). I am now working on the 'from decimal' conversions. I have the dec_to_bin method working correctly, as well. My issue is with the dec_to_hex method. I want it to use the dec_to_bin method to convert the string to binary first and then use the bin_to_hex method for the final conversion. In doing this it has to overlook the lines of code that tell the 2 methods to display their results; but rather to store the results and transport them to the dec_to_hex method.

I'm going to post the entire code except for the method that creates the Tkinter widgets:

def base_check(self):
    """ Disable Checkbox that's connected with chosen Radiobutton. """
    sel_radio = self.base.get()

    for radio, cb in self.cb_to_radio.items():
        if radio == sel_radio:
            cb.configure(state = DISABLED)
        else:
            cb.configure(state = NORMAL)       

def conv_segue(self):
    """ Decides and directs towards proper conversion method. 
        Reading the 'convert from' radiobuttons. """
    base = self.base.get()

    if base == 'bin':
        bits = self.input_str.get()

        # test string validity        
        bit_list = list(bits)
        ill_bits = ['2', '3', '4', '5', '6', '7', '8', '9']
        for bit in bit_list:
            if bit in ill_bits:
                self.output_disp.delete(0.0, END)
                self.output_disp.insert(0.0, "That bit string is invalid.")
                break
        else:        
            self.from_binary(self.dec_bttn, self.hex_bttn)

        ##
        # learned here that I had to break once the match was found (if found) and that a 'for'
        # loop can use an else block too
        ##

    elif base == 'dec':
        self.from_dec(self.bin_bttn, self.hex_bttn)
    elif base == 'hex':
        self.from_hex(self.bin_bttn, self.dec_bttn)

def from_binary(self, dec_bttn, hex_bttn):
    """ Finds what base to convert to (Decimal or Hex) from binary. """

    if self.dec_bttn.get():
        self.bin_to_dec()          

    if self.hex_bttn.get():
        self.bin_to_hex()

    #if self.dec_bttn.get() and self.hex_bttn.get():
        #(self.bin_to_dec, self.bin_to_hex)

def from_dec(self, bin_bttn, hex_bttn):
    """ Finds what base to convert to (Binary or Hex) from decimal. """

    if self.bin_bttn.get():
        self.dec_to_bin()

    if self.hex_bttn.get():
        self.dec_to_hex()

def dec_to_bin(self):
    """ Convert from decimal to binary. """

    # get input string and convert to an integer
    digits = self.input_str.get()
    digits = int(digits)

    bit_string = ""

    # do the conversion
    while digits:
        bit, ans = digits%2, digits//2
        bit = str(bit)
        bit_string += bit
        digits = ans

    total = bit_string[::-1]
    self.total = total

    # print output
    self.print_result(total)
    #return total



def dec_to_hex(self):
    bit_str = self.dec_to_bin().self.total
    self.bit_str = bit_str
    print(self.bit_str)


def bin_to_dec(self):
    """ Convert from binary to decimal. """
    # get input string
    bits = self.input_str.get()

    # set exponent
    exp = len(self.input_str.get()) - 1

    tot = 0

    # do conversion
    while exp >= 1:
        for i in bits[:-1]:
            if i == '1':
                tot += 2**exp
            elif i == '0':
                tot = tot                        
            exp -= 1

        if bits[-1] == '1':
            tot += 1

        total = tot
        # print output
        self.print_result(total)
        #return total

def bin_to_hex(self):
    """ Convert from binary to hex. """
    # get input string
    bits = self.input_str.get()

    # define hex digits
    hex_digits = {
                10: 'a', 11: 'b',
                12: 'c', 13: 'd',
                14: 'e', 15: 'f'
                }

    # add number of necessary 0's so bit string is multiple of 4
    string_length = len(bits)
    number_stray_bits = string_length % 4

    # test if there are any 'stray bits'
    if number_stray_bits > 0:       
        number_zeros = 4 - number_stray_bits
        bits = '0'*number_zeros + bits
        string_length = len(bits)

    # index slicing positions
    low_end = 0
    high_end = 4

    total = ""

    # slice bit string into half byte segments
    while high_end <= string_length:
        exp = 3
        half_byte = bits[low_end:high_end]

        # do conversion
        tot = 0
        while exp >= 1:
            for i in half_byte[:-1]:

                if i == '1':
                    tot += 2**exp

                elif i == '0':
                    tot = tot                        
                exp -= 1

            if half_byte[-1] == '1':
                tot += 1

        # check if tot needs conversion to hex digits           
        for i in hex_digits.keys():
            if i == tot:
                tot = hex_digits[i]
            else:
                tot = tot

        # store and concatenate tot for each while iteration             
        tot = str(tot)
        total += tot 

        # move right to next half byte string       
        low_end += 4
        high_end += 4

    # print the output            
    self.print_result(total)
    #return total

def print_result(self, total):
    """ display the result of conversion. """
    self.output_disp.delete(0.0, END)
    self.output_disp.insert(0.0, total)

I've tried to make it as easy to read for anyone who attempts to help me as possible. The dec_to_hex method is a bit of a mess right now, I have been messing with it. With the code posted I think its a bit more clear what exactly I'm trying to do. Its very simple code as I haven't a lot of Python experience.

Its all in one class, and I'm trying to do it without the need to copy the code from the two methods I want to use for the dec_to_hex method (dec_to_bin & bin_to_hex).

I thought about breaking it into different classes and using inheritance, but I can't see where that will help me at all

My final decision to post the question now while I continue to mess with it came because I'm sure once its figured out I'm going to have learned something very important, and probably a concept or two that I don't have a complete grasp on will become clearer.

I hope someone will be willing to give me a little direction in this matter. I get an adrenalin rush when I see progress getting made. Its also very well commented and docstringed so it shouldn't be a problem for anyone.

I also thought that this would be a great situation to use some equivalent to the XHTML anchors, but then I realized they are about the same as the old goto command from my 6th grade BASIC days and figured Python was too 'clean' to use that.

\$\endgroup\$
1
  • \$\begingroup\$ Kill dead code and variables; from_binary arguments dec_bttn and hex_bttn are useless and should be removed. \$\endgroup\$
    – Chris Morgan
    Commented Feb 7, 2012 at 9:57

4 Answers 4

6
\$\begingroup\$

Use more python, e.g. instead of:

for i in hex_digits.keys():
    if i == tot:
        tot = hex_digits[i]
    else:
        tot = tot

Remove extraneous keys() and it becomes:

for i in hex_digits:
    if i == tot:
        tot = hex_digits[i]
    else:
        tot = tot

Then remove unnecessary else and it is:

for i in hex_digits:
    if i == tot:
        tot = hex_digits[i]

And finally remove the loop:

tot = hex_digits.get(i, tot)

There I saved you a loop, a branch and 4 out of 5 lines of code.

A few iterations like this over the entire module and you might like your code after all!

\$\endgroup\$
3
  • \$\begingroup\$ Thanks, I'll go through it and clean it up a bit. My problem is I don't prep with any pseudo-code, I just kinda wing it as I go. Terrible habit, I know. \$\endgroup\$
    – Icsilk
    Commented Feb 7, 2012 at 10:00
  • \$\begingroup\$ @Icsilk, no pseudo code is a terrible habit. I don't know of any serious coders who actually use it. \$\endgroup\$ Commented Feb 7, 2012 at 15:39
  • \$\begingroup\$ I write pseudocode from time to time, and I'd consider myself a serious (albeit not professional) coder... but I certainly wouldn't say not using pseudocode is a terrible habit. \$\endgroup\$
    – David Z
    Commented Feb 7, 2012 at 21:51
2
\$\begingroup\$
def base_check(self):
    """ Disable Checkbox that's connected with chosen Radiobutton. """
    sel_radio = self.base.get()

    for radio, cb in self.cb_to_radio.items():
        if radio == sel_radio:
            cb.configure(state = DISABLED)
        else:
            cb.configure(state = NORMAL)       

def conv_segue(self):
    """ Decides and directs towards proper conversion method. 
        Reading the 'convert from' radiobuttons. """
    base = self.base.get()

    if base == 'bin':
        bits = self.input_str.get()

        # test string validity        
        bit_list = list(bits)

It's not neccessary to listify the bits. You can iterate over a string just like a list.

        ill_bits = ['2', '3', '4', '5', '6', '7', '8', '9']

I'd make this a string rather then a list and iterate over that.

        for bit in bit_list:
            if bit in ill_bits:

Instead, I'd use if any(bit in ill_bits for bit in bit_list). Also, why don't you check for bits that are 1 or 0, rather then explicitly listing the other options. What if the user inputs a letter?

                self.output_disp.delete(0.0, END)
                self.output_disp.insert(0.0, "That bit string is invalid.")
                break
        else:        
            self.from_binary(self.dec_bttn, self.hex_bttn)

        ##
        # learned here that I had to break once the match was found (if found) and that a 'for'
        # loop can use an else block too
        ##

    elif base == 'dec':
        self.from_dec(self.bin_bttn, self.hex_bttn)
    elif base == 'hex':
        self.from_hex(self.bin_bttn, self.dec_bttn)

I don't see this function anywhere

Why do you perform sanity checks for binary, and not the other bases?

def from_binary(self, dec_bttn, hex_bttn):
    """ Finds what base to convert to (Decimal or Hex) from binary. """

Why are you ussing dec_bttn and hex_bttn around if you just use self.dec_bttn anyways?

    if self.dec_bttn.get():
        self.bin_to_dec()          

    if self.hex_bttn.get():
        self.bin_to_hex()

    #if self.dec_bttn.get() and self.hex_bttn.get():
        #(self.bin_to_dec, self.bin_to_hex)

def from_dec(self, bin_bttn, hex_bttn):
    """ Finds what base to convert to (Binary or Hex) from decimal. """

    if self.bin_bttn.get():
        self.dec_to_bin()

    if self.hex_bttn.get():
        self.dec_to_hex()

def dec_to_bin(self):
    """ Convert from decimal to binary. """

    # get input string and convert to an integer
    digits = self.input_str.get()
    digits = int(digits)

    bit_string = ""

    # do the conversion
    while digits:
        bit, ans = digits%2, digits//2
        bit = str(bit)
        bit_string += bit
        digits = ans

    total = bit_string[::-1]

Python has a function, bin that does this conversion to binary for you.

    self.total = total

    # print output
    self.print_result(total)
    #return total



def dec_to_hex(self):
    bit_str = self.dec_to_bin().self.total

What?

    self.bit_str = bit_str
    print(self.bit_str)

I'm not seeing the hex.

def bin_to_dec(self):
    """ Convert from binary to decimal. """
    # get input string
    bits = self.input_str.get()

    # set exponent
    exp = len(self.input_str.get()) - 1

    tot = 0

    # do conversion
    while exp >= 1:
        for i in bits[:-1]:
            if i == '1':
                tot += 2**exp
            elif i == '0':
                tot = tot                        
            exp -= 1

        if bits[-1] == '1':
            tot += 1

        total = tot
        # print output
        self.print_result(total)
        #return total

Use int(string_number, 2) to read in a binary number.

def bin_to_hex(self):
    """ Convert from binary to hex. """
    # get input string
    bits = self.input_str.get()

    # define hex digits
    hex_digits = {
                10: 'a', 11: 'b',
                12: 'c', 13: 'd',
                14: 'e', 15: 'f'
                }

    # add number of necessary 0's so bit string is multiple of 4
    string_length = len(bits)
    number_stray_bits = string_length % 4

    # test if there are any 'stray bits'
    if number_stray_bits > 0:       
        number_zeros = 4 - number_stray_bits
        bits = '0'*number_zeros + bits
        string_length = len(bits)

Python has an rjust method on strings that'll pad strings to a desired length. I think you can simplify this code using that.

    # index slicing positions
    low_end = 0
    high_end = 4

    total = ""

    # slice bit string into half byte segments
    while high_end <= string_length:

You should really use a for loop like for high_end in xrange(0, string_length, 4): exp = 3 half_byte = bits[low_end:high_end]

        # do conversion
        tot = 0

Don't abbreviate variables. It saves you almost nothing and makes your code harder to read.

        while exp >= 1:

This doesn't really serve a purpose because exp is decremented by the inner loop.

            for i in half_byte[:-1]:

I'd use for exponent, letter in enumerate(halt_byte[::-1]).

                if i == '1':
                    tot += 2**exp

                elif i == '0':
                    tot = tot   

Completely pointless. Doesn't assign variables to themselves

                exp -= 1

            if half_byte[-1] == '1':
                tot += 1

Why didn't you do this in the loop: exp**0 = 1

        # check if tot needs conversion to hex digits           
        for i in hex_digits.keys():
            if i == tot:
                tot = hex_digits[i]
            else:
                tot = tot

Its a dictionary. don't use a loop on it. Put all of the numbers in it, not just the non-digit ones. Then use tot = hex_digits[tot]

        # store and concatenate tot for each while iteration             
        tot = str(tot)
        total += tot 

I'd combine those two lines

        # move right to next half byte string       
        low_end += 4
        high_end += 4

If you use a for loop like I suggested this should be unneccesary.

Actually python has a hex function which will convert a number to hex. It'll replace pretty much this entire function.

    # print the output            
    self.print_result(total)
    #return total

def print_result(self, total):
    """ display the result of conversion. """
    self.output_disp.delete(0.0, END)
    self.output_disp.insert(0.0, total)

This function doesn't really print. So I'd find a better name.

Here's how I'd approach it.

def convert_base(number_text, from_base, to_base):
    BASES = {
         'decimal' : 10,
         'hex' : 16,
         'binary', 2)
    number = int(number_text, BASES[from_base])

    if to_base == 'decimal':
       return str(number)
    elif to_base == 'hex':
       return hex(number)[2:]
    elif to_base == 'binary':
       return bin(number)[2:]
    else:
       raise ValueError('Unknown base: ' + base)

In general conversions work best by converting to some neutral format, (in this case, a python integer), and then into your final format. That way you don't have to write conversion between every possible format. Instead, you just a conversion for each format into the neutral format and then out of the neutral format.

\$\endgroup\$
1
\$\begingroup\$

you could pass a second parameter to dec_to_bin and dec_to_hex to let the function know whether you want to return a value or print it.

def dec_to_bin(self,ret=0):

    ....
    if ret == 0:
        self.print_result(total)
    else:
        return total

Then call the function like:

binstr = dec_to_bin(1) #to assign the return value to binstr
\$\endgroup\$
1
  • 3
    \$\begingroup\$ When python has True and False built in, using numbers to approximate them seems odd... \$\endgroup\$
    – Shish
    Commented Feb 7, 2012 at 12:11
0
\$\begingroup\$

I'd split out the x_to_y methods into a separate package, and rather than having them do all of input, processing, output, have them just do processing.

def hex_to_bin(hex_input):
    ... processing goes here ...
    return bin_output

And then in your GUI app, you have the input and output:

hex_input = self.input_box.get_value()
bin_output = hex_to_bin(hex_input)
self.output_box.set_value(bin_output)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.