Need some constructive criticism on code

A forum for general discussion of the Python programming language.

Need some constructive criticism on code

Postby skywola » Sun Aug 18, 2013 1:29 am

I have just completed this GUI for the main window of my app, and I am almost positive there is
an indentation problem, but I am not sure how to fix it. Other than that, there may be issues
with the import statements (I had trouble getting this thing to run without all the code at the
top, and, I can't tell whether I am running Python 3.3 or 2.7, because I have them both installed.
I think I am running the newer version, 3.3) Which one uses tk, and which one uses TK?
So in summary, the issues are:
1. Indentation
2. Import statements
3. Version, tk vs Tk.

By the way, the buttons on the right are meant to be blank, they will be dynamically filled, I am just
re-building an application that I originally wrote in Visual Basic 2010 in Python. I'm new to Python,
have been working with it for about a week. Waa hoo! :geek:

Code: Select all
import sys, Tkinter
import tkFont
sys.modules['tkinter'] = Tkinter
import os
from tkinter import *

root = Tk()
root.customFont = tkFont.Font(family="Times New Roman", size=18)


def Rainbow():
    filewin = Toplevel(root)
   
root.minsize(958,200)
#root.maxsize(1920,980)
root.maxsize(1800,900)
RWidth=root.winfo_screenwidth()
RHeight=root.winfo_screenheight()
root.geometry(("%dx%d")%(RWidth-24,RHeight-24))
root.geometry('+2+2')
root.configure(background='darkgrey')

menubar = Menu(root)
filemenu = Menu(menubar, tearoff=0)
filemenu.add_command(label="New")
filemenu.add_command(label="Open")
filemenu.add_command(label="Save")
filemenu.add_command(label="Save as...")
filemenu.add_separator()
filemenu.add_command(label="Print")
filemenu.add_command(label="Print Preview")
filemenu.add_command(label="Page Setup")
filemenu.add_separator()
filemenu.add_command(label="Import Searches")
filemenu.add_command(label="Export Searches")
filemenu.add_separator()
filemenu.add_command(label="Exit", command=root.quit)
menubar.add_cascade(label="File", menu=filemenu)

editmenu = Menu(menubar, tearoff=0)
editmenu.add_command(label="Undo")
editmenu.add_command(label="Redo")
editmenu.add_separator()
editmenu.add_command(label="Insert Image")
editmenu.add_separator()
editmenu.add_command(label="Cut")
editmenu.add_command(label="Copy")
editmenu.add_command(label="Paste")
editmenu.add_command(label="Select All")
editmenu.add_command(label="Find")
editmenu.add_command(label="Replace")
menubar.add_cascade(label="Edit", menu=editmenu)

formatmenu = Menu(menubar, tearoff=0)
formatmenu.add_command(label="Word Wrap")
formatmenu.add_command(label="Font")
formatmenu.add_command(label="Options")
formatmenu.add_command(label="Backcolor")
formatmenu.add_separator()
formatmenu.add_command(label="Align")
formatmenu.add_command(label="Indent")
formatmenu.add_separator()
formatmenu.add_command(label="Superscript")
formatmenu.add_command(label="Subscript")
formatmenu.add_separator()
formatmenu.add_command(label="Bold")
formatmenu.add_command(label="Italics")
formatmenu.add_command(label="Underline")
menubar.add_cascade(label="Format", menu=formatmenu)

helpmenu = Menu(menubar, tearoff=0)
helpmenu.add_command(label="Help Index")
helpmenu.add_command(label="Go To Website")
helpmenu.add_command(label="Keyborad Map")
helpmenu.add_separator()
helpmenu.add_command(label="About")
menubar.add_cascade(label="Help", menu=helpmenu)



# Draw top row of buttons
btnPPreviewImage = PhotoImage(file='ppreview.gif')
btnPPreview = Button(root, image=btnPPreviewImage)
btnPPreview.place(bordermode=OUTSIDE, height=30, width=30)

btnPrintImage = PhotoImage(file='Print.gif')
btnPrint = Button(root, image=btnPrintImage)
btnPrint.place(bordermode=OUTSIDE, height=30, width=30, x=32)

btnCapDeCapImage = PhotoImage(file='CapDeCap.gif')
btnCapDeCap = Button(root, image=btnCapDeCapImage)
btnCapDeCap.place(bordermode=OUTSIDE, height=30, width=30, x=64)

btnIncreaseImage = PhotoImage(file='Increase.gif')
btnIncrease = Button(root, image=btnIncreaseImage)
btnIncrease.place(bordermode=OUTSIDE, height=30, width=30, x=96)

btnSectionImage = PhotoImage(file='Section.gif')
btnSection = Button(root, image=btnSectionImage)
btnSection.place(bordermode=OUTSIDE, height=30, width=30, x=128)



# Draw a frame to limit size (height) of search textbox
frame = Frame(root, width=222, height=29, bg="grey", relief=RIDGE)
frame.place(x=160, y=0)
# put search textbox in frame
frame.txtBox = Text(frame, width = 22, height = 2, bg="lightgrey", font=("Times New Roman",18))
frame.txtBox.place(x=0, y=0)




# Row 2 Buttons
btnSaveImage = PhotoImage(file='Save.gif')
btnSave = Button(root, image=btnSaveImage)
btnSave.place(bordermode=OUTSIDE, height=30, width=30, x=0, y=32)

btnBoldImage = PhotoImage(file='Bold.gif')
btnBold = Button(root, image=btnBoldImage)
btnBold.place(bordermode=OUTSIDE, height=30, width=30, x=32, y=32)

btnItalicsImage = PhotoImage(file='Italics.gif')
btnItalics = Button(root, image=btnItalicsImage)
btnItalics.place(bordermode=OUTSIDE, height=30, width=30, x=64, y=32)

btnUnderlineImage = PhotoImage(file='Underline.gif')
btnUnderline = Button(root, image=btnUnderlineImage)
btnUnderline.place(bordermode=OUTSIDE, height=30, width=30, x=96, y=32)

btnSearchFirstImage = PhotoImage(file='SearchFirst.gif')
btnSearchFirst = Button(root, image=btnSearchFirstImage)
btnSearchFirst.place(bordermode=OUTSIDE, height=30, width=30, x=128, y=32)

btnSearchLastImage = PhotoImage(file='SearchLast.gif')
btnSearchLast = Button(root, image=btnSearchLastImage)
btnSearchLast.place(bordermode=OUTSIDE, height=30, width=30, x=160, y=32)

btnFindFowardImage = PhotoImage(file='FindForward.gif')
btnFindFoward = Button(root, image=btnFindFowardImage)
btnFindFoward.place(bordermode=OUTSIDE, height=30, width=30, x=192, y=32)

btnFindReverseImage = PhotoImage(file='FindReverse.gif')
btnFindReverse = Button(root, image=btnFindReverseImage)
btnFindReverse.place(bordermode=OUTSIDE, height=30, width=30, x=224, y=32)

btnReturnImage = PhotoImage(file='Return.gif')
btnReturn = Button(root, image=btnReturnImage)
btnReturn.place(bordermode=OUTSIDE, height=30, width=30, x=256, y=32)

btnLMarkerImage = PhotoImage(file='LMarker.gif')
btnLMarker = Button(root, image=btnLMarkerImage)
btnLMarker.place(bordermode=OUTSIDE, height=30, width=30, x=288, y=32)

btnColorImage = PhotoImage(file='Color.gif')
btnColor = Button(root, image=btnColorImage)
btnColor.place(bordermode=OUTSIDE, height=30, width=30, x=320, y=32)

#btnBlankImage = PhotoImage(file='x.gif')
btnBlank = Button(root, text="BL") #image=btnColorImage)
btnBlank.place(bordermode=OUTSIDE, height=30, width=30, x=352, y=32)






# Create Frame for Color Panels
# Draw a frame to limit size (height) of textbox
#colorBackPanel = Frame(root, width=226, height=62, bg="grey", relief=RIDGE)
#colorBackPanel.place(x=386, y=0)

# Draw Color Panel Buttons
btnC0 = Button(root, bg="red")
btnC0.place(bordermode=OUTSIDE, height=30, width=30, x=386, y=1)
btnC1 = Button(root, bg="tomato")
btnC1.place(bordermode=OUTSIDE, height=30, width=30, x=386, y=32)
btnC2 = Button(root, bg="green4")
btnC2.place(bordermode=OUTSIDE, height=30, width=30, x=418, y=1)
btnC3 = Button(root, bg="green1")
btnC3.place(bordermode=OUTSIDE, height=30, width=30, x=418, y=32)
btnC4 = Button(root, bg="blue3")
btnC4.place(bordermode=OUTSIDE, height=30, width=30, x=450, y=1)
btnC5 = Button(root, bg="BlueViolet")
btnC5.place(bordermode=OUTSIDE, height=30, width=30, x=450, y=32)
btnC6 = Button(root, bg="chocolate1")
btnC6.place(bordermode=OUTSIDE, height=30, width=30, x=482, y=1)
btnC7 = Button(root, bg="cyan")
btnC7.place(bordermode=OUTSIDE, height=30, width=30, x=482, y=32)
btnC8 = Button(root, bg="brown")
btnC8.place(bordermode=OUTSIDE, height=30, width=30, x=514, y=1)
btnC9 = Button(root, bg="burlywood")
btnC9.place(bordermode=OUTSIDE, height=30, width=30, x=514, y=32)
btnC10 = Button(root, bg="yellow")
btnC10.place(bordermode=OUTSIDE, height=30, width=30, x=546, y=1)
btnC11 = Button(root, bg="pink")
btnC11.place(bordermode=OUTSIDE, height=30, width=30, x=546, y=32)
btnC12 = Button(root, bg="DimGrey")
btnC12.place(bordermode=OUTSIDE, height=30, width=30, x=578, y=1)
btnC13 = Button(root, bg="aquamarine")
btnC13.place(bordermode=OUTSIDE, height=30, width=30, x=578, y=32)


btnWhite = Button(root, bg="white")
btnWhite.place(bordermode=OUTSIDE, height=30, width=30, x=612, y=0)
btnBlackout = Button(root, bg="black")
btnBlackout.place(bordermode=OUTSIDE, height=30, width=30, x=612, y=32)


# Draw Radio Buttons
# Create Frame for Radio Buttons
radioBtnPanel = Frame(root, width=116, height=62, bg="lightgrey", relief=RIDGE)
radioBtnPanel.place(x=646, y=0)
v = IntVar()
#W = Radiobutton(root, font=("Times New Roman",14))
root.customFont = tkFont.Font(family="Times New Roman", size=13)
Radiobutton(root, text="English", font=root.customFont, variable=v, value=1, bg="lightgrey").place(x=650, y=2)
Radiobutton(root, text="Russian", font=root.customFont, variable=v, value=2, bg="lightgrey").place(x=650, y=30)
root.customFont = tkFont.Font(family="Times New Roman", size=18)


# Create Special Character Buttons
btnSpChar0 = Button(root)
btnSpChar0.place(bordermode=OUTSIDE, height=30, width=30, x=766, y=0)

btnSpChar1 = Button(root)
btnSpChar1.place(bordermode=OUTSIDE, height=30, width=30, x=766, y=32)

btnSpChar2 = Button(root)
btnSpChar2.place(bordermode=OUTSIDE, height=30, width=30, x=798, y=0)

btnSpChar3 = Button(root)
btnSpChar3.place(bordermode=OUTSIDE, height=30, width=30, x=798, y=32)

btnSpChar4 = Button(root)
btnSpChar4.place(bordermode=OUTSIDE, height=30, width=30, x=830, y=0)

btnSpChar5 = Button(root)
btnSpChar5.place(bordermode=OUTSIDE, height=30, width=30, x=830, y=32)

btnSpChar6 = Button(root)
btnSpChar6.place(bordermode=OUTSIDE, height=30, width=30, x=862, y=0)

btnSpChar7 = Button(root)
btnSpChar7.place(bordermode=OUTSIDE, height=30, width=30, x=862, y=32)

btnSpChar8 = Button(root)
btnSpChar8.place(bordermode=OUTSIDE, height=30, width=30, x=894, y=0)

btnSpChar9 = Button(root)
btnSpChar9.place(bordermode=OUTSIDE, height=30, width=30, x=894, y=32)

btnCloseImage = PhotoImage(file='Close.gif')
btnClose = Button(root, image=btnCloseImage, command=lambda:root.destroy())
btnClose.place(bordermode=OUTSIDE, height=30, width=30, x=926, y=0)

root.txtBox = Text(root, bg="lightgrey", font=("Times New Roman",18))

# RWidth=root.winfo_screenwidth()
# RHeight=root.winfo_screenheight()
root.txtBox.place(x=4, y=68, relwidth=1, relheight=1, width=-10, height=-76) #width=root.winfo_screenwidth()-44, height=root.winfo_screenheight()-44)  #width=RWidth-44, height=RHeight-44)





root.config(menu=menubar)
root.mainloop()
skywola
 
Posts: 3
Joined: Tue Aug 13, 2013 1:38 am

Re: Need some constructive criticism on code

Postby metulburr » Sun Aug 18, 2013 1:41 am

Code: Select all
import sys, Tkinter
import tkFont
sys.modules['tkinter'] = Tkinter
import os
from tkinter import *

This is just plain old going to fail as it is importing both 3.x and 2.x tkinter modules without try/except to catch an error for the other version.
Also for the star imports check the tutorials here regarding that.

If you dont know which python version you are running you can read this. If you are using an IDE, and just not sure which version the IDE is using, you are probably going to have to indicate the IDE, for us to navigate that specific IDE to its path its using. Especially if you plan to change it. Although an easy quick way to self-test it, is just have a single line in the IDE import both
Code: Select all
import Tkinter
and
Code: Select all
import tkinter
and see which ones causes an error, indicating the version used. CApitalized Tkinter is python 2.x, and lowercase tkinter is python3.x

I dont see why you would have an indentation problem as you have only ONE indented line in the entire code.

by the way, there has to be major changes to the code to even get it to run, as no one on this forum has magically all those gifs in a directory with those names. Its too much a pain.
New Users, Read This
version Python 3.3.2 and 2.7.5, tkinter 8.5, pyqt 4.8.4, pygame 1.9.2 pre
OS Ubuntu 14.04, Arch Linux, Gentoo, Windows 7/8
https://github.com/metulburr
User avatar
metulburr
 
Posts: 1112
Joined: Thu Feb 07, 2013 4:47 pm
Location: Elmira, NY

Re: Need some constructive criticism on code

Postby skywola » Sun Aug 18, 2013 2:35 am

Ok, Thanks for the feedback!!! . . . .

So it turns out I am running Python 3.3 . . .

I think you are saying that you should not use the * on the import line.

Would the following work?
Code: Select all
try:
    import Tkinter as Tk
except Exception:
    sys.exc_clear()
    import tkinter as tk


If I have this in the top lines:
Code: Select all
import sys
import Tkinter as Tk
import tkFont


Isn't tkFont part of tkinter, that is, from python 2.7? If that is the case, I am not using the right import statement
for my fonts. I suspect it should be "TkTextFont", but I tried that and it was not recognized:
Code: Select all
root.customFont = TkTextFont.Font(family="Times New Roman", size=18)


Also, is the indentation ok in the code I posted earlier?
.
skywola
 
Posts: 3
Joined: Tue Aug 13, 2013 1:38 am

Re: Need some constructive criticism on code

Postby Mekire » Sun Aug 18, 2013 6:26 am

Theoretically 2.7 and 3.x compatible imports:
Code: Select all
import sys
import os

try:
    import Tkinter as tk
    import tkFont as tkfont
except ImportError:
    import tkinter as tk
    import tkinter.font as tkfont

Then you just use tk for tkinter and tkfont for tkFont and it should work in both. As Metul said though we can't actually test your code because it relies on so many resources and is a pain to change.

-Mek

Edit: Capitalization error. Fixed.
User avatar
Mekire
 
Posts: 820
Joined: Thu Feb 07, 2013 11:33 pm
Location: Amakusa, Japan

Re: Need some constructive criticism on code

Postby skywola » Sun Aug 18, 2013 6:50 pm

Ok, thanks for the help, metulburr and mekire . . .

I have created a dumned-down version by removing all the images and clutter so
anyone can run it.

If you set the beginning variable called fail equal to 0, it will cruise right
thru and show the GUI, but if you set it to 1, it gives the error


try
Traceback (most recent call last):
File "C:/Python33/Rainbow/Test4.py", line 48, in <module>
root = Tk()
TypeError: 'module' object is not callable


As you may notice, I put print statements in each branch with the name of
the branch so you can tell where it fails or succeeds!

It seems that somewhere along the line the
Code: Select all
root = Tk()

is not right or not in the right place, or perhaps an issue with tkFont.

Here is the code:

Code: Select all
import sys, os

fail = 0
if fail ==1:
    try:
        import Tkinter as Tk   
        import tkFont as tkFont
        print('try')
    except ImportError:
        import tkinter as Tk
        import tkinter.font as tkFont
        print('except')
else:
    from Tkinter import *
    import tkFont
    print('else')

#Start normal code
def doGui():
    root.customFont = tkFont.Font(family="Times New Roman", size=18)
    root.minsize(958,200)
    root.maxsize(1800,900)
    RWidth=root.winfo_screenwidth()
    RHeight=root.winfo_screenheight()
    root.geometry(("%dx%d")%(RWidth-24,RHeight-24))
    root.geometry('+2+2')
    root.configure(background='darkgrey')

    # Draw a few Color Panel Buttons
    btnC0 = Button(root, bg="red")
    btnC0.place(bordermode=OUTSIDE, height=30, width=30, x=386, y=1)
    btnC1 = Button(root, bg="tomato")
    btnC1.place(bordermode=OUTSIDE, height=30, width=30, x=386, y=32)
    btnC2 = Button(root, bg="green4")
    btnC2.place(bordermode=OUTSIDE, height=30, width=30, x=418, y=1)
    btnC3 = Button(root, bg="green1")
    btnC3.place(bordermode=OUTSIDE, height=30, width=30, x=418, y=32)

    btnClose = Button(root, text="Close", command=lambda:root.destroy())
    btnClose.place(bordermode=OUTSIDE, height=30, width=40, x=926, y=0)

    root.txtBox = Text(root, bg="lightgrey", font=("Times New Roman",18))
    root.txtBox.place(x=4, y=68, relwidth=1, relheight=1, width=-10, height=-76)
    root.mainloop()


if __name__=="__main__":
    root = Tk()
    doGui()
skywola
 
Posts: 3
Joined: Tue Aug 13, 2013 1:38 am

Re: Need some constructive criticism on code

Postby Mekire » Mon Aug 19, 2013 12:01 am

You have been councelled not to use the star import. This means that anytime you access any part of tkinter (given the previous imports you need to write the tk prefix.

Firsly I would suggest you change your import so that Tk is not capitalized:
Code: Select all
import tkinter as tk
but you need to realize this exception business is only there for purposes of running in python 2 and 3. If you are only planning on using 3, don't bother.

if you have:
Code: Select all
import tkinter as tk #the python 3 version
then you need to write:
Code: Select all
tk.Tk()

-Mek
User avatar
Mekire
 
Posts: 820
Joined: Thu Feb 07, 2013 11:33 pm
Location: Amakusa, Japan


Return to General Discussions

Who is online

Users browsing this forum: No registered users and 1 guest