4
\$\begingroup\$

My script encodes text and outputs random strings. It is different each time and loads a dictionary that has been pre-set by my dictionary gen.py (included after the main script). I've started my basic Tkinter GUI and this is not the finishing GUI. The main code is as follows:

# -*- coding: utf-8 -*-

import random, pickle
from Tkinter import *
from tkinter.font import Font
root = Tk()

def error(msg):
    result.delete("1.0", END)
    result.insert(END, msg)

def encode(string):
    y = random.randrange(0, 200)
    idNo = y + 40
    idNo = idNo * 7
    idNo = idNo - 32
    idNo = idNo * 6
    idNo = idNo + 3
    idNo = str(idNo)
    idLength = 4
    idLetters = list(idNo)
    base = 'dict/base.txt'
    with open(str(base), "rb") as fp:
        dictionary = pickle.load(fp)
    for i in xrange(0, int(idLength)):
        letter = idLetters[0]
        value = dictionary[str(letter)]
        idLetters.remove(letter)
        idLetters.append(value)
    idNo = ''.join(idLetters)

    firstidNo = idNo[:2]
    lastidNo = idNo[-2:]

    letters = list(string)
    length = len(string)
    chosendict = "dict/" + str(y) + ".txt"
    with open(str(chosendict), "rb") as fp:
        dictionary = pickle.load(fp)
    for i in xrange(0, int(length)):
        letter = letters[0]
        value = dictionary[str(letter)]
        letters.remove(letter)
        letters.append(value)

    encoded = ''.join(letters)
    text = str(firstidNo) + encoded + str(lastidNo)
    original = decode(text)
    if original != string:
        return ("Error: Encoding Unsuccessful\nPlease Contact Whoever gave this program")
    else:
        return text

def decode(inputdecode):
    firstidNo = inputdecode[:2]
    lastidNo = inputdecode[-2:]
    idNo = str(firstidNo) + str(lastidNo)
    base = 'dict/rbase.txt'
    with open(str(base), "rb") as fp:
        dictionary = pickle.load(fp)
    idLength = 4
    idLetters = list(idNo)
    for i in xrange(0, int(idLength)):
        letter = idLetters[0]
        value = dictionary[str(letter)]
        idLetters.remove(letter)
        idLetters.append(value)
    idNo = ''.join(idLetters)
    idNo = int(idNo)
    idNo = idNo - 3
    idNo = idNo / 6
    idNo = idNo + 32
    idNo = idNo / 7
    idNo = idNo - 40
    chosendict = "dict/r" + str(idNo) + '.txt'
    with open(str(chosendict), "rb") as fp:
        dictionary = pickle.load(fp)
    string = inputdecode[:-2]
    string = string[2:]
    letters = list(string)


    length = len(string)
    for i in xrange(0, int(length)):
        letter = letters[0]
        value = dictionary[str(letter)]
        letters.remove(letter)
        letters.append(value)

    decoded = ''.join(letters)
    text = decoded
    return text

def guiencode():
    string = encoding.get("1.0","end-1c")
    text = encode(string)
    result.delete("1.0", END)
    result.insert(END, text)

def guidecode():
    inputdecode = decoding.get("1.0","end-1c")
    text = decode(inputdecode)
    result.delete("1.0", END)
    result.insert(END, text)

root.title('Encoder V2.0')

timesNewRoman = Font(family="Arial", size=10)
height1 = 2
width1 = 40

encoding = Text(root, height=height1, width=width1)
encoding.pack()
encoding.configure(font=timesNewRoman)
buttonToEncode = Button(root, text = "Encode", command=lambda: guiencode()).pack(pady=2)
decoding = Text(root, height=height1, width=width1)
decoding.pack(pady=3)
decoding.configure(font=timesNewRoman)
buttonToDecode = Button(root, text = "Decode", command=lambda: guidecode()).pack(pady=2)
result = Text(root, height=height1, width=width1)
result.pack()
result.configure(font=timesNewRoman)


root.mainloop()

As you can see it chooses a dictionary and then through a non-changing dictionary it encodes the id for that dictionary. As there are 200 dictionaries the number it generates (idNo) is always 4 digits so it splits it into 2 and puts two at the front and two at the back.

I also use a gen.py file which generates my dictionary files:

# -*- coding: utf-8 -*-

import random, pickle
all_chars = ['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z',
             'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',
             '!','"','$','%','^','&','*','(',')','_','-','+','=','{','}','[',']',':',';','@',"'",'~','#','<','>',',','.','/','?','|',
             '1','2','3','4','5','6','7','8','9','0',' ']
all_chars2 = ['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z',
             'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',
             '!','"','$','%','^','&','*','(',')','_','-','+','=','{','}','[',']',':',';','@',"'",'~','#','<','>',',','.','/','?','|',
             '1','2','3','4','5','6','7','8','9','0',' ']
dictionary1 = {}
dictionary2 = {}

length = len(all_chars)
print(length)

for x in range(0, 201):
    random.shuffle(all_chars2)
    for i in xrange(0, 93):
        dictionary1[all_chars[i]] = str(all_chars2[i])
        dictionary2[all_chars2[i]] = str(all_chars[i])
    #print(dictionary1)
    #print(dictionary2)
    #print('dictionary' + str(x) + str(dictionary1))
    dicname = 'dict/' + str(x) +'.txt'
    with open(str(dicname), "wb") as fp:
        pickle.dump(dictionary1, fp)
    dicname = 'dict/r' + str(x) + '.txt'
    with open(str(dicname), "wb") as fp:
        pickle.dump(dictionary2, fp)

random.shuffle(all_chars2)
for i in xrange(0, 93):
    dictionary1[all_chars[i]] = str(all_chars2[i])
    dictionary2[all_chars2[i]] = str(all_chars[i])
#print(dictionary1)
#print(dictionary2)
#print('dictionary' + str(x) + str(dictionary1))
dicname = 'dict/base.txt'
with open(str(dicname), "wb") as fp:
    pickle.dump(dictionary1, fp)
dicname = 'dict/rbase.txt'
with open(str(dicname), "wb") as fp:
    pickle.dump(dictionary2, fp)

Anything I can do to improve my code is appreciated as well as features that would be nice for me to add (Things like encoding/decoding history etc.)

Thanks!

NB/ I know I overuse int() and str(), its just a habit I'm in to stop me getting errors!

EDIT:

A picture of the current GUI

MY GUI

\$\endgroup\$
2
\$\begingroup\$

The improvements I would like to suggest are related to Tkinter only:

Starting with the imports, do not use wildcard imports. I mean, instead of coding from Tkinter import *, write import Tkinter as Tk. Then you will need to prefix all the widgets you use with Tk. The main interest of this approach is that in case you want to use the themed widgets, you will not have namespaces clashes as described here.

In history, graphical user interfaces spurred the object-oriented design. So it is nice you to use a class to build your GUI because, as described here, this gives you a private namespace for all of your callbacks and private functions, and just generally makes it easier to organize your code. In a procedural style you have to code top-down, defining functions before using them, etc. With this method you don't since you don't actually create the main window until the very last step. I prefer inheriting from tk.Frame just because I typically start by creating a frame, but it is by no means necessary.

I suggest you to create a class

import Tkinter as Tk 


class MyGUI(Tk.Frame): # Please use a better class name

    def __init__(self, master):
        Tk.Frame.__init__(self, master)
        self.master = master
        self.configure_gui()
        self.create_widgets()

This is a good design. All what you have to do now is to complete coding the configure_gui() and create_widgets() functions above. For instance, all the settings of your interface should be put in the configuration function like follows:

def configure_gui(self):
   self.master.title('Encoder V2.0')
   # continue if any ...

Then create the widgets as follows:

def create_widgets(self):
    self.encoding = Tk.Text(self.master, height=..., width=...)
    # Note how I replace "root" by "self.master"
    # Use better names, "encoding" is not a good name to instantiate a Text widget.
\$\endgroup\$
2
\$\begingroup\$

I don't have time to review all of your code (at least for now), but here's some recommendations:

  • If I'm not mistaken, xrange starts at 0 if no first argument is given (this is the case with range in Python 3). So unless you need to start at another initial value, leave out the first argument and just call xrange(<upper bound>). The same goes for random.randrange, by the way.

  • Some of your variable names are not descriptive. In particular dictionary1 and dictionary2, and all_chars2. As I didn't go through all of your code, I have no clue why you're using two seperate dictionaries, but you'd probably be able to get rid of the second one by rewriting your program slightly.

  • Make use of the string module:

    >>> import string
    >>> string.ascii_lowercase
    'abcdefghijklmnopqrstuvwxyz'
    >>> string.ascii_uppercase
    'ABCDEFGHIJKLMNOPQRSTUVWXYZ'
    >>> string.digits
    '0123456789'
    >>> string.punctuation
    '!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~'
    
  • There's no need to cast the filename to a string:

    dicname = 'dict/base.txt'
    # Anyone remotely familiar with Python understands that dicname is a `str`
    with open(str(dicname), "wb") as fp:
    # Confusing, because this makes it seem as though it's not a `str`
    
\$\endgroup\$
  • 2
    \$\begingroup\$ thanks, if only I knew about the string module before, could have saved myself ages... \$\endgroup\$ – KCG Jul 9 '17 at 16:12

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.