LinuxQuestions.org
Visit Jeremy's Blog.
Home Forums Tutorials Articles Register
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices


Reply
  Search this Thread
Old 07-21-2016, 10:06 AM   #1
leprechaun3
LQ Newbie
 
Registered: May 2016
Posts: 20

Rep: Reputation: Disabled
Please critique my python3.4/tkinter code (short program)


This is a die roller program I wrote to learn python. Everything does work, but I do intend on making some modifications, mostly just to make the output easier to read. The first version simply rolled a single die with a user defined number of sides and gave the output in a label, so I think I have come a long way since then considering this is my first python program.
I'm not a part of a class or having any formal training in programming and know few people that do any programming. So I'm curious if there are better ways of doing things than what I have done, both looking a functionality of the final program and the quality of the code itself.

Are there things that I have grouped together and should have separated?
Are there things that I separated and should have grouped together?
Are there shorter ways of accomplishing the same functionality (without having to completely re-write it to make changes later)?

Again I am wanting to make some changes, but I thought that I would go ahead and post this before the code got too long.

Code:
#!/usr/bin/python3.4
import tkinter as tk
from tkinter import ttk
from random import randrange

def roll_loop(bonus, die, sides):
	try:
		total = int(0)
		if bonus != 0: #Check if a bonus should be added
			#The loop runs 1 less time than the number of dice
			#This is done for formating
			for totalrolls in range(0, die):
				rolled = int(randrange(1, sides)) #Calculate die roll
				result_entry.insert(tk.END, rolled) #These print the results
				result_entry.insert(tk.END, " + ")
				result_entry.insert(tk.END, bonus)
				result_entry.insert(tk.END, " + ")
				rollplusbonusloop = int(rolled + bonus) #Add the bonus
				total += rollplusbonusloop #Generate a cumulative total
			else:
				#Run the calculation loop one more time
				#This gets to the appropriate number of dice
				#While keeping formating
				rolled = int(randrange(1, sides))
				result_entry.insert(tk.END, rolled)
				result_entry.insert(tk.END, " + ")
				result_entry.insert(tk.END, bonus)
				result_entry.insert(tk.END, " = ")
				rollplusbonusloop = int(rolled + bonus)
				total += rollplusbonusloop
				return total #Return the final total of all rolls
		else:
			#Run this loop if no bonus is to be added
			#Otherwise done the same as above loop
			#Separated for formating
			for totalrolls2 in range(0, die):
				rolled = int(randrange(1, sides))
				result_entry.insert(tk.END, rolled)
				result_entry.insert(tk.END, " + ")
				total += rolled
			else:
				rolled = int(randrange(1, sides))
				result_entry.insert(tk.END, rolled)
				result_entry.insert(tk.END, " = ")
				total += rolled
				return total
	except ValueError:
		pass

def give_total(*args):
	try:
		#Pull numbers for calculations
		addbonus = int(bonus.get())
		numberofdice = int(die.get())
		numberofdice -= 1 #Subtracted for formating in the loops
		numberofsides = int(sides.get())
		numberofsides += 1 #Added to give full range of possibilities
		#Calculate total by calling the appropriate loop
		total = int(roll_loop(addbonus, numberofdice, numberofsides))
		#Print total, equal sign printed at end of loops prior to this
		result_entry.insert(tk.END, total)
		result_entry.insert(tk.END, "\n") #Add line break at end of rolls
	except ValueError:
		pass

#Create root
root = tk.Tk()
root.title("Die Roller")

#List of variables
sides = tk.StringVar()
result = tk.StringVar()
die = tk.StringVar()
bonus = tk.StringVar()

#Create main window in root
mainframe = ttk.Frame(root, padding="10")
mainframe.grid(column=0, row=0, sticky=(tk.N, tk.W, tk.E, tk.S))

#Create labels
ttk.Label(mainframe, text="Sides").grid(column=4, row=1, sticky=tk.NW)
ttk.Label(mainframe, text="Results").grid(columnspan=6, row=3, sticky=tk.NE)
ttk.Label(mainframe, text="Dice").grid(column=1, row=1, sticky=tk.NW)
ttk.Label(mainframe, text="Bonus").grid(column=6, row=1, stick=tk.NW)
#These labels are for showing order of operations to user
ttk.Label(mainframe, text="X").grid(column=2, row=2, sticky=tk.NW)
ttk.Label(mainframe, text="+").grid(column=5, row=2, sticky=tk.NW)
ttk.Label(mainframe, text="(").grid(column=3, row=2, sticky=tk.NW)
ttk.Label(mainframe, text=")").grid(column=7, row=2, sticky=tk.NW)

#Create buttons
ttk.Button(mainframe, text="Roll", command=give_total).grid(column=8, row=2, sticky=tk.NW)


#Create entry boxes
side_entry = ttk.Entry(mainframe, width=3, textvariable=sides)
side_entry.grid(column=4, row=2, sticky=tk.NW)
die_entry = ttk.Entry(mainframe, width=3, textvariable=die)
die_entry.grid(column=1, row=2, sticky=tk.NW)
die_entry.insert(0, "1")
bonus_entry = ttk.Entry(mainframe, width=3, textvariable=bonus)
bonus_entry.grid(column=6, row=2, sticky=tk.NW)
bonus_entry.insert(0, "0")

#Create a frame to house the result field and scrollbars
result_frame = tk.Frame(mainframe, width=53, height=13)
result_frame.grid(columnspan=9, row=5)

#Create scrollbars for results field
resultyscroll = tk.Scrollbar(result_frame)
resultyscroll.grid(column=2, row=1, sticky=tk.W+tk.N+tk.S)
resultxscroll = tk.Scrollbar(result_frame, orient=tk.HORIZONTAL)
resultxscroll.grid(column=1, row=2, sticky=tk.N+tk.W+tk.E)

#Create text field for listing results
result_entry = tk.Text(result_frame, width=50, height=10, wrap="none")
result_entry.grid(column=1, row=1)
result_entry.config(xscrollcommand=resultxscroll.set, yscrollcommand=resultyscroll.set)

#Connect scrollbars with text field now that both are created
resultyscroll.config(command=result_entry.yview)
resultxscroll.config(command=result_entry.xview)

for child in mainframe.winfo_children(): child.grid_configure(padx=5, pady=5)

#Give focus to side_entry on open
side_entry.focus()
#Bind the return/enter key to give the "give_total" command
root.bind('<Return>', give_total)

root.mainloop()
Thank you
 
Old 07-21-2016, 10:31 AM   #2
HMW
Member
 
Registered: Aug 2013
Location: Sweden
Distribution: Debian, Arch, Red Hat, CentOS
Posts: 773
Blog Entries: 3

Rep: Reputation: 369Reputation: 369Reputation: 369Reputation: 369
Hi!

At a quick glance I would say ot looks pretty ok.

I have a tip for you; try using pylint. It actually gets you out of bad habits pretty quickly!
If you don't want to install that, you can check your code online:
http://pep8online.com/

Your code, for instance, produced this:
http://pep8online.com/s/xpdNxR9a


Here's a quick suggestion (you'll thank me later) do use a docstring for ALL your functions. Like this:
Code:
def foo(aString, anInt):
    """
    Print aString anInt times
    """

    […]

Best regards,
HMW
 
Old 07-21-2016, 12:19 PM   #3
grail
LQ Guru
 
Registered: Sep 2009
Location: Perth
Distribution: Manjaro
Posts: 10,006

Rep: Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191Reputation: 3191
I ran it and it works ... so great start

As nothing to be passed to the script obviously not really a help command required to tell you anything, however, I found that if I blanked / deleted values from one of the entry points (say 'dice'),
instead of being told that it cannot be empty or that empty does not equate to 0 it simply sits there and looks at me and i have no way of knowing if anything happened until I try again. This is
of course being a little pedantic, but some people really like to be spoon fed what is going on

Another usery type thing, if I drag the window to try and see more of the results assuming I picked say 10 dice and want to see all on screen at once, it appears the outer shell gets dragged and made bigger
but not the inner window which is attached to it

Last one from me ... you created the variable 'result', but as far as I can see it is never used. I am not a python guru so it may be needed??

Great job for first time
 
Old 07-21-2016, 12:33 PM   #4
leprechaun3
LQ Newbie
 
Registered: May 2016
Posts: 20

Original Poster
Rep: Reputation: Disabled
Thank you the suggestions.

HMW: I had not heard of pep8online or the docstrings. It seems the only complaint from pep8 is with the spacing, but it still runs. I'll still try to fix those just to tidy things up. I just used gedit for writing this and run it in terminal to find errors, there is a gedit plugin for using pylint so I'll try to get that set up as it will be faster than constantly copying code into pep8.

Grail: I have not put in any error messages yet, I probably should start doing that so that it becomes habit. With the resizing I just haven't bothered looking up how to get that to work, what I initially saw for using grid in tkinter was that it should just work, but obviously it doesn't. And the result variable had been used before, but moved away from it when adding in the bonus option and apparently I just didn't delete it from the code.
 
  


Reply



Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off



Similar Threads
Thread Thread Starter Forum Replies Last Post
Short Tutorial : How to install Code Aster in Slackware64 13 Alexvader Slackware 6 12-21-2009 04:22 PM
python, tkinter, command is executed when program is run not when button clicked deathalele Programming 5 06-15-2009 03:13 PM
code critique? a better way? dave201 Programming 8 06-25-2007 04:32 AM
Error in basic button response program in Python 2.4 with the Tkinter module jojotx0 Programming 1 05-23-2006 07:43 PM
What is the problem with this short code? Mistro116@yahoo.com Programming 3 11-13-2005 09:54 AM

LinuxQuestions.org > Forums > Non-*NIX Forums > Programming

All times are GMT -5. The time now is 07:57 AM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
Open Source Consulting | Domain Registration