[Tutor] Shortening code [was Re: Tutor Digest, Vol 93, Issue 117]

[Alan Gauld]

Please change to a sensible subject when replying to digest messages.

On 20/11/11 11:45, Mic wrote:
 > I have done my best to make the code more readable now.
 > Here is the result.

############################
from tkinter import*

button1_color="green"
button1_value=False

button2_color="green"
button2_value=False
############################

buttonX_value still doesn't say much about *why* the variable is there. 
What are you storing in it. What does the value represent?

Also, do you really need the colors, you don't actually use them for 
anything below except the initial color, but you might as well just hard 
code it to "green", it would be shorter and more explicit...

##################################
class Window(Frame):
    def __init__(self,master):...

    def create_widgets(self):

        #Creates hello button1
        self.hello_bttn1=Button(self,bg=button1_color,
                            text="Hi_1", command=self.button1_clicked)
        self.hello_bttn1.grid()

        #Creates hello button2
        self.hello_bttn2=Button(self,bg=button2_color,
                            text="Hi_1", command=self.button2_clicked)
        self.hello_bttn2.grid()

    def button1_clicked(self):
        """ This method runs if button one is clicked"""

        def change_button1_value():
            global button1_value
            button1_value=not button1_value

        change_button1_value()
############################

 >        if button1_value:
 >            self.hello_bttn1.configure(bg="red", text="Hi_2")

Note that this is the line that really changes the button's color.
And it does not use the variable...

 >           def change_button1_color_red():
 >               global button1_color
 >               button1_color=("red")
 >           change_button1_color_red()

Whereas this function and its call do nothing but change the value of a 
variable that is never used after the initial creation. If you took this 
out the code would run with exactly the same effect.

 >       else:
 >           self.hello_bttn1.configure(bg="green", text="Hi_1")

Same, here. You set the color explicitly, then create
a function and call it just to update a variable you don't use.

 >           def change_button1_color_green():
 >               global button1_color
 >               button1_color=("green")
 >           change_button1_color_green()


###############################
    def button2_clicked(self):
        """This method runs if button two is clicked"""

        def change_button2_value():
            global button2_value
            button2_value=not button2_value

        change_button2_value()

        if button2_value:

            self.hello_bttn2.configure(bg="red", text="Hi_2")

            def change_button2_color_red():
                global button2_color
                button2_color=("red")
            change_button2_color_red()

        else:
            self.hello_bttn2.configure(text="Hi_1", bg="green")

            def change_button2_color_green():
                global button2_color
                button2_color=("green")
            change_button2_color_green()
############################################


Notice that both button handlers are identical except they work on 
different buttons and test values. What we'd like is a single
function that gets passed the widget and test as parameters.
It would look like this:

def button_clicked(self, widget, test):
     if test:
         widget.configure(bg="red", text="Hi_2")
     else:
         widget.configure(text="Hi_1", bg="green")


and to call it we create two short event handlers:

def button1_clicked(self):
      self.button1_value = not self.button1_value
      self.button_clicked(button1,self.button1_value)


def button2_clicked(self):
      self.button2_value = not self.button2_value
      self.button_clicked(button2,self.button2_value)


Notice I've moved the test values to instance variables
rather than globals, but thats just a style thing you could have 
referenced the globals within the handlers instead.

I also removed those spurious functions from the main
button_clicked code.


 > However, I did not understand this part of your suggestions:

 > > Generally you build a data table with all the config parameters that
 > > will varty then you build a loop to read the values from the
 > > data table.
 > > Store the created buttons in a list or dictionary.
 >
 > I must admit that I have never heard of a "data table" before.
 > Is this easy to do, for a beginner like me?


Yes its just a list of tuples/lists. For your two buttons uit would look 
like:

#  buttonName text, color, command
self.button_params = [
("b1", "Hi_1", "green", self.button1_clicked),
("b2", "Hi_2", "green", self.button2_clicked)
]

Now your create_widgets looks like:

    def create_widgets(self):
        self.Buttons = {}
        for params in self.button_params:
            b = Button(self, bg=params[1],
                       text=params[2], command=params[3])
            self.Buttons[params[0]] = b


And now you can access your buttons at any time using

self.Buttons["b1"]

etc

We could use a dictionary for params to make it more readable at the 
expense of more typing:

#  buttonName text, color, command
self.button_params = [
{
   "name": "b1",
   "text":"Hi_1",
   "color":"green",
   "Command":self.button1_clicked
},
{
    "name": "b1",
    "text":"Hi_1",
    "color":"green",
    "Command":self.button1_clicked
}
]

You could then put that in a separate file which you import into you GUI 
creation file. You could have a similar list for each type of widget you 
want to create. However you still need to know how to place them within 
the GUI, so either you create a very complex parameters structure or you 
just keep the complexity in the creation method.
But tis technique is worth while where you are creating a list of near 
identical widgets - like the buttons in a calculator keybad say...

To process the dictionary the creation loop changes to:

        for params in self.button_params:
            b = Button(self, bg=params["color"],
                       text=params["text"], command=params["command"])
            self.Buttons[params["name"]] = b

Hopefully that makes it clear.


-- 
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/