[Tutor] Shortening code.

[Mic]

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

-Yes, sorry about that.


############################
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...

------------------------------------------------------------

This is a part of a larger program I am writing, where you are supposed to 
book chairs on a train.
Button1 is supposed to represent chair one in the train. When you click at 
the button, which represents a chair,
it is supposed to change color to show that the chair now is booked. If you 
press it one more time, it is supposed to
change color to show that the chair is free to book.
That was the idea. I am sorry if this was confusing, I will try to explain 
it more detailed later on in the mail.

-----------------------------------------------------------------------------------------
>        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()

---------------------------------------------------------------------
I understand why you think I don't need to change the global value, since I 
am already changing the color with this line.
  self.hello_bttn1.configure(bg="green", text="Hi_1")

The thing is that this window I have created is just a window in another 
window. I want the color to be the same that it was before I closed the 
window.
Otherwise the color would be green when I open the window again, despite 
closing the window when the button was red. I hope you understand what I 
mean by this?

###############################
    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.

-------------------------------------------------------------------------

This was exactly what I thought, that they these functions are so similiar 
that it should be one way to
shorten this code. In reality, in my larger program, I have about 10 buttons 
that should change value,
and colors when you click them.


> 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

----------------------------------------------------------------------------------


Thank you for this detailed explaination. I have tried to get your 
suggestions above to work
but I feel I should explain a little more detailed why I am asking these 
questions. As I mentioned
at the top of my email, I am writing a larger program. This program is 
supposed to be a GUI
online booking program for train tickets. I want to move up on year in 
school so I am trying to finish
this program that is supposed to be finished in a long, long time.

These buttons I have created above are supposed to represent a chair. So 
when you click
at one of the chairs  it is supposed to change color from green to red, and 
at the same time
create a file with the chair's number, price and so on. I have managed to 
create this file without
any problem. But the thing is that I have written a function for each of the 
chairs that does nearly the same
thing. That is not good, because I want one function to do this for all 
chairs.

I know you tried to explain this, and you did it very good, it is just that 
I don't understand how I should implement
this in my program.



If you want to I could send you my real program, and you would perhaps get a 
better understanding of what I am
trying to accomplish and why I ask these questions.

However, I understand if you don't have time to answer these probably stupid 
and simple questions, I am still
grateful for your previous answers!

Thank you!

Mic