9
\$\begingroup\$

This is supposed to encode and decode small strings of text. Unfortunately, even on a really good laptop, it performs slowly. This might have to do with my looping, or just the sheer computation.

It takes a string, uses simple RSA, subtracts by random #, converts both to fixed binary, then compresses using base 80 "everyletter". To decrypt, it just reverses the process.

The encryption and decryption use binary numbers of length 5 and 19, so it is very important they don't dynamically change.

Take a look and feel free to comment on anything else that seems wrong:

# P, M, and D are initialized here

def encode_list( string, m, p ):
    prog.config(maximum=len(string), value=0)
    l = list(string)
    code = []
    for c in l:
        code.append(encode(c, m, p))
        prog.step()
        win.update()
    return code

def encode( ch, m, p ):
    return (ord(ch)**p) % m

def decode_list( num_list ):
    prog.config(maximum=len(num_list), value=0)
    trans_nums = []
    word = []
    for q in num_list:
        trans_nums.append(decode(q))
        prog.step()                                      #for a progress bar, usually more than 5 sec for 30+ characters
        win.update()
    for r in trans_nums:
        word.append(chr(r))
    return word

def decode( num ):
    return (num**D) % M

def to_binary( number, base ):                        #I need a fixed length base for repeatability
    r = ""
    temp = number
    powlis = []
    for power in range(base-1, -1, -1):
        n = pow(2, power)
        powlis.append(n)
    for p in powlis:
        if temp-p >= 0:
            r+="1"
            temp-=p
        else:
            r+="0"
    return r

def from_binary( bi_string, base ):
    r = 0
    bitlist = []
    for bit in bi_string:
        bitlist.append(int(bit))
    bitlist.reverse()
    for add in range(base-1, -1, -1):
        r += pow(2, add)*bitlist[add]
    return r

everyletter=[]

for x in range(1, 11):
    everyletter.append(str(x)[len(str(x))-1])
for x in range(97, 123):
    everyletter.append(chr(x))
for x in range(65, 91):
    everyletter.append(chr(x))
for char in ['~','!','@','#','$','%','^','&','[',']','{','}','|',';',':',',','.','?']:
    everyletter.append(char)

def to_everyletter( num ):            #special thing I made up, used for compression
    temp=num
    x=0
    y=-1
    while temp>=x:
        y+=1
        x=pow(len(everyletter), y)
    output = ''
    for q in range(y-1, -1, -1):
        mult=1
        while temp>=mult*pow(len(everyletter), q):
            mult+=1
        temp-=(mult-1)*pow(len(everyletter), q)
        output+=everyletter[mult-2]
    return output

def from_everyletter( lets ):
    maxpow=len(lets)-1
    output = 0
    rev = lets[::-1]
    for q in range(maxpow, -1, -1):
        output+=(everyletter.index(rev[q])+1)*pow(len(everyletter), q)
    return output


computers = {
    "myself":[M, P]             #usually contains more, as if you could send to other people
}

from tkinter import *
import tkinter.ttk as ttk
import tkinter.filedialog as filedialog
import random
from time import sleep
import os

win = Tk()
win.iconbitmap(default='logo.ico')     #somewhere else
win.title("RSA encryption-python")
entrybar = Frame(win)
buttons = Frame(win)
file_list = Frame(win)

s_entry = Text(entrybar, width=25, height=5)
input_label = Label(entrybar, text="Input")
prog = ttk.Progressbar(entrybar, mode="determinate")
input_label.pack(pady=5)
s_entry.pack(expand=1,fill=BOTH,padx=5)
prog.pack(padx=20, side=BOTTOM, expand=1, fill=X)

filename = StringVar()
f_entry = Entry(buttons, textvariable=filename)
people = Listbox(buttons, selectmode=BROWSE)
for person in computers:
    people.insert(END, person)

en_files = Listbox(file_list, selectmode=BROWSE)

class I_have_to:
    def __init__(self):
        self.path=os.getcwd()
derp = I_have_to()

def refresh():
    files=[]
    en_files.delete(0, END)
    for file in os.listdir(derp.path):
        if file.endswith(".bef2"):
            files.append(file)
    for file in files:
        en_files.insert(END, file)

def refresh_path():
    root = Tk()
    root.withdraw()
    directory = filedialog.askdirectory(parent=root, initialdir="/", title="Select the folder with Encrypted files")
    derp.path = str(directory)
    root.destroy()
    refresh()

file_label = Label(file_list, text="Select file:")
file_list2 = Frame(file_list)
path_refresh = Button(file_list2, text="Choose folder", command=refresh_path)
file_label.pack(pady=3)
en_files.pack()
path_refresh.pack(padx=5,pady=2)
file_list2.pack()

def encode_text():
    s = s_entry.get(1.0, END)[0:len(s_entry.get(1.0, END))-1]
    v1 = computers[people.get(ACTIVE)][0]
    v2 = computers[people.get(ACTIVE)][1]
    code = encode_list( s, v1, v2 )
    print(code)
    key_list = []
    for thi in range(0, len(code)):
        key_list.append(random.randint(0, 31))
        code[thi] += key_list[thi]
    output = ""
    for counter in range(0, len(key_list)):
        binary = str(to_binary(key_list[counter], 5))
        binary += str(to_binary(code[counter], 19))
        output += to_everyletter(from_binary(binary,24))+'_'

    f = open(derp.path + "/" + filename.get() + ".bef2", 'w')
    f.write(output)
    f.close()
    refresh()

def decode_text():
    f = open(derp.path + "/" + en_files.get(ACTIVE))
    g = f.read()
    f.close()
    elist=g.split('_')
    blist=[]
    for ever in elist:
        blist.append(to_binary(from_everyletter(ever), 24))
    h=''.join(blist)
    if not(len(h) % 24 == 0):
        s_entry.insert(END,"Error: wrong number of bits")
        return None
    teh = []
    key_list = []
    for q in range(0, int(len(h)/24)+1):
        key_list.append(h[q*24:q*24+5])
        teh.append(h[q*24+5:(q+1)*24])
    del teh[len(teh)-1]
    del(key_list[len(key_list)-1])
    dec1 = []
    for counter in range(0, len(key_list)):
        dec1.append(from_binary(teh[counter], 19))
        key_list[counter] = from_binary(key_list[counter], 5)
        dec1[counter] -= key_list[counter]
    dec2 = decode_list(dec1)
    f = "".join(dec2)
    s_entry.insert(END, f)

encrypt = Button(buttons, text="Encode", command=encode_text)
decrypt = Button(buttons, text="Decode", command=decode_text)
encrypt.pack(padx=10, pady=2)
decrypt.pack(padx=10, pady=2)
f_entry.pack()
people.pack()

entrybar.pack(side=LEFT, expand=1, fill=BOTH, pady=5, padx=5)
buttons.pack(side=LEFT)
file_list.pack(side=LEFT, padx=5)
refresh()
win.mainloop()
quit()

Download (a little bit old) can be found here

\$\endgroup\$
  • \$\begingroup\$ Consider using numpy arrays (docs.scipy.org/doc/numpy/reference/generated/numpy.array.html) instead of lists of integers. If it's still too slow, it might be time to break out the C compiler and write an extension. \$\endgroup\$ – bsa Jan 23 '15 at 1:33
  • \$\begingroup\$ Welcome to Code Review! It looks like you've come to the right place to help speed up working code. I hope you get some good reviews! \$\endgroup\$ – Phrancis Jan 23 '15 at 1:47
4
\$\begingroup\$
  • Please note that I am just commenting general readability not performance

You really like single letter variable names:

  • l = list(string)
  • encode( ch, m, p )
  • r = ""

Please, use longer variable names to enhance readability.


Quoting from the good answer of Janne and adding:

Try to separate computations into functions with clear inputs and outputs

Also you may create two files:

  1. compress.py in which you will write all your algoritmhs of compression.
  2. gui.py in which you write the Tkinter code for the user interface.

This allows simpler maintaining: if you see that the encoding is broken go to that file, if you think that the GUI is broken go to the GUI file.


Also, it would be a good idea to add some explanation about what the program is doing and what algorithm it is using.

To be more precise, in Python there is a widely used convention called docstrings, they look like this:

def double(x):
    """Returns the double of x"""
    return 2*x

from tkinter import *

This is acceptable for tkinter but it is more readable to use:

import tkinter as tk

As people reading will instantly know when something is a tkinter class.


Now let's analyze the following block of code:

everyletter=[]

for x in range(1, 11):
    everyletter.append(str(x)[len(str(x))-1])
for x in range(97, 123):
    everyletter.append(chr(x))
for x in range(65, 91):
    everyletter.append(chr(x))
for char in ['~','!','@','#','$','%','^','&','[',']','{','}','|',';',':',',','.','?']:
    everyletter.append(char)

running this gives:

 ['1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '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', '~', '!', '@', '#', '$', '%', '^', '&', '[', ']', '{', '}', '|', ';', ':', ',', '.', '?']

So you can simplify your code a lot if you declare EVERY_LETTER as a constant: at the start of the file write:

EVERY_LETTER = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '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', '~', '!', '@', '#', '$', '%', '^', '&', '[', ']', '{', '}', '|', ';', ':', ',', '.', '?']

But the name is wrong: '4' '}' and ';' for example are not letters, they are called 'chars' (that is a technical term short for characters). so it would be better to call it ALL_CHARS.


Performance

Use PyPy. It give drastic speed-ups with no code change.

\$\endgroup\$
  • \$\begingroup\$ I've never thought about putting my code in separate files...thanks for the tip! \$\endgroup\$ – Blue Jan 23 '15 at 15:14
  • \$\begingroup\$ Make sure to put an __init__.py file in the folder so you can import from one file to the other. \$\endgroup\$ – Tom Jan 23 '15 at 16:36
2
\$\begingroup\$
  • Let Python do the work of converting numbers to and from binary:

    binary = '{:0{}b}'.format(number, width)
    number = int(binary, 2)
    
  • pow(num, D, M) is a faster way of computing (num**D) % M

  • I was able to give the above advice because a part of your code is organized into functions with a clear purpose. Unfortunately the same can't be said about all the code. You have mixed computations with user interface code. Try to separate computations into functions with clear inputs and outputs. Also, it would be a good idea to add some explanation about what the program is doing and what algorithm it is using.
\$\endgroup\$
  • \$\begingroup\$ Thank you for the answer! I edited my question to show you what it does. I also should probably make a separate function for the updating of the progress bar, like you said. And I like those faster functions! \$\endgroup\$ – Blue Jan 23 '15 at 11:55
1
\$\begingroup\$

A bit of a late nitpick. The array here:

for x in range(1, 11):
    everyletter.append(str(x)[len(str(x))-1])
for x in range(97, 123):
    everyletter.append(chr(x))
for x in range(65, 91):
    everyletter.append(chr(x))
for char in ['~','!','@','#','$','%','^','&','[',']','{','}','|',';',':',',','.','?']:
    everyletter.append(char)

['~','!','@','#','$','%','^','&','[',']','{','}','|',';',':',',','.','?'] is a bit painful to read. Since a string is an array of chars, you can write it like this:

ALL_CHARS = "~!@#$%^&[]{}|;:,.?"

Even better, since this is now a string, you can write this:

[...]
for x in range(65, 91):
        everyletter.append(chr(x))
everyletter += "~!@#$%^&[]{}|;:,.?"
\$\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.