[Tutor] Iterators, example (need help)

[Alan Gauld]

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/