Reputation: 341
I have written a function that is going to have up to 72 IF statements and i was hoping to write code that will be much shorter, but have no idea where to start
The function reads the self.timeselect variable when a radio button is selected and the result is saved to a text file called missing_time.txt. If the result is equal to 1 then save "0000" to the file, if the result is 2 save then 0020 to the text file etc. This can be for 72 possible combinations.
Is there a smarter way to simplify the function ?
def buttonaction():
selectedchoice = ""
if self.timeselect.get() == 1:
selectedchoice = "0000"
orig_stdout = sys.stdout
f = open('missing_time.txt', 'w')
sys.stdout = f
print(selectedchoice)
f.close()
if self.timeselect.get() == 2:
selectedchoice = "0020"
orig_stdout = sys.stdout
f = open('missing_time.txt', 'w')
sys.stdout = f
print(selectedchoice)
f.close()
self.timeselect = tkinter.IntVar()
self.Radio_1 = tkinter.Radiobutton(text="0000",variable =
self.timeselect,indicator = 0 ,value=1)
self.Radio_1.place(x=50,y=200)
self.Radio_2 = tkinter.Radiobutton(text="0020",variable =
self.timeselect,indicator = 0 ,value=2)
self.Radio_2.place(x=90,y=200)
Upvotes: 1
Views: 9843
Reputation: 10211
choice_map = {
1 : "0000",
2 : "0020"
}
def buttonaction():
selected = self.timeselect.get()
if 0 < selected < 73: # This works as intended in Python
selectedchoice = choice_map[selected]
# Do you intend to append to file instead of replacing it?
# See text below.
with open("missing_time.txt", 'w') as outfile:
outfile.write(selectedchoice + "\n")
print(selectedchoice)
Better yet, if there is a pattern that relates the value of self.timeselect.get()
to the string that you write out, generate selectchoice
directly from that pattern instead of using a dictionary to do the mapping.
I find it a bit odd that you are clearing the file "missing_time.txt" every time you call buttonaction
. If your intention is to append to it, change the file mode accordingly.
Also, instead of opening and closing the file each time, you might just want to open it once and pass the handler to buttonaction or keep it as a global depending on how you use it.
Finally, if you do not intend to catch the KeyError from an invalid key, you can do what @Clifford suggests and use choice_map.get(selected, "some default value that does not have to be str")
.
Upvotes: 6
Reputation: 93476
All you need to do in this case is construct a string from the integer value self.timeselect.get()
.
selectedchoice = self.timeselect.get()
if 0 < selectedchoice < 73:
orig_stdout = sys.stdout
f = open('missing_time.txt', 'w')
sys.stdout = f
print( str(selectedchoice).zfill(4) ) # Convert choice to
# string with leading
# zeros to 4 charaters
f.close()
Further in the interests of simplification, redirecting stdout and restoring it is a cumbersome method of outputting to a file. Instead, you can write directly to the file:
with open('missing_time.txt', 'w') as f:
f.write(selectedchoice + "\n")
Note that because we use the with
context manager here, f
is automatically closed when we leave this context so there is no need to call f.close()
. Ultimately you end up with:
selectedchoice = self.timeselect.get()
if 0 < selectedchoice < 73:
with open('missing_time.txt', 'w') as f:
f.write( str(selectedchoice).zfill(4) + "\n" )
Even if you did use the conditionals each one differs only in the first line, so only that part need be conditional and the remainder of the content performed after the conditionals. Moreover all conditionals are mutually exclusive so you can use else-if
:
if self.timeselect.get() == 1:
selectedchoice = "0000"
elif self.timeselect.get() == 2:
selectedchoice = "0020"
...
if 0 < selectedchoice < 73:
with open('missing_time.txt', 'w') as f:
f.write(selectedchoice + "\n")
In circumstances where there is no direct arithmetic relationship between selectchoice
and the required string, or the available choices are perhaps not contiguous, it is possible to implement a switch
using a dictionary:
choiceToString = {
1: "0001",
2: "0002",
...
72: "0072",
}
selectedchoice = choiceToString.get( self.timeselect.get(), "Invalid Choice")
if selectedchoice != "Invalid Choice":
with open('missing_time.txt', 'w') as f:
f.write(selectedchoice + "\n")
Upvotes: 3
Reputation: 23
I'm assuming that the values are arbitrary and there's no defined pattern. I also see that the only thing that changes in your code is the selectedChoice variable. You can use a Dictionary in such cases. A dictionary's elements are key/value pairs so you can reference the key and get the value.
dictionary = {
1:"0000",
2:"0020",
3:"0300",
4:"4000"
}
def buttonAction():
selectedChoice = dictionary[self.timeselect.get()]
if 0<selectedChoice<=72:
f=open('missing_time.txt','w')
f.write(selectedChoice+" ")
f.close()
print(choice)
Upvotes: 0
Reputation: 88
Since there is no switch statement in Python, you can't really reduce the number of if statements. But I see 2 two way to optimize and reduce your code length.
First, you can use some
if condition:
elif condition:
instead of
if condition:
if condition:
since you can't have self.timeselect.get() evaluated to more than one int. Secondly you can wrap all the code that doesn't vary in a function. You can get rid of selectedchoice and put
orig_stdout = sys.stdout
f = open('missing_time.txt', 'w')
sys.stdout = f
print(selectedchoice)
f.close()
in a function writeToFile(selectedOption)
Upvotes: 0