On 23/10/12 00:32, Prasad, Ramit wrote:
Most of Ramit's comments are valid, this is just a couple of additional
notes.
from Tkinter import *
This is a frowned upon import style as it can easily override existing
names. Instead use:
import Tkinter as tk # You can change "tk" to something else.
import * is common for Tkinter apps, but its still better in production
code to use the '...as tk' style.
import os
ignore1='''
This program works to do what I set out to do as a first step
BAZ 10/19/2012
'''
You don't need the ignore= bit.
Just make it a doc string in the code.
class Env:
''' This class represents the environment of the current process'''
def __init__(self, master=None):
self.d = os.environ.keys()
self.v = os.environ.values()
self.i = 0
self.y = self.v[self.i]
self.x = self.d[self.i]
def __iter__(self):
self.x = self.d[self.i]
self.y = self.v[self.i]
return self
Environment variables do not seem like something that really
needs to be iterable. Although, if I was going to do that
I would instead just store os.environ dictionary (instead
of keys and values) and iterate over that.
Agreed, I'd rather store the dict directly in the class.
What are s1 and s2? Why should they be global? Why not return
here directly? What happens if the key is None?
They are defined in the later code, but I agree they don't
need to be global. Accessing values across classes is bad
practice. It would be better that if you really want
s1,s2 to be global that you define them in global scope
outside any of the class definitions. It makes them easier
to find!
else:
print ("raising Stop Iteration")
>> raise StopIteration
I assume its only for debugging but printing a message and raising an
exception should not be done. Its for the exception handling code to
decide if a message is needed.
class App(object):
''' This is the main GUI app'''
def __init__(self,master):
self.createWidgets()
def createWidgets(self,master=None):
global s1
global s2
self.fm1 = Frame(master)
self.fm2 = Frame(master)
self.fm1.pack(side = TOP, anchor=NW, fill=BOTH,expand=YES)
self.fm2.pack(side = TOP, anchor=SW, fill=BOTH,expand=YES)
s1 and s2 should be passed in and not be a global variable.
If you need to update the data later, then create an update
method instead of looking at global variables.
s1=StringVar()
s1.set(env.x)
env is not set in the code and so I have no idea what this refers to.
It's in the main() function but not declared as global here which is
inconsistent and confusing. Again better to put it into the class.
I suppose by the time this gets created there is an env in the global
scope, but I think this is chancy and will only work from this script.
You cannot reuse or import class App from another program.
Instead pass env into the __init__ method.
I have not run this program, but I am pretty sure this button will
not update e1 and e2. Not to mention that you need to store e1/e2
somewhere you can access later.
This is a bit of Tkinter magic that means the entries are auto updated
when the variable values change (and vice versa). That's what a
StringVar does. So I think you are Ok here.
HTH,
--
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
_______________________________________________
Tutor maillist - Tutor@python.org
To unsubscribe or change subscription options:
http://mail.python.org/mailman/listinfo/tutor
