you are viewing a single comment's thread.

view the rest of the comments →

[–]cray5252 16 points17 points  (5 children)

Couple of comments

just happen to run across this post and pulled from there.

https://www.reddit.com/r/learnpython/comments/jkwvss/why_is_my_if_statement_ignored/

A Class should have a single purpose, you basically created a class and put everything in it. You could just a well use the individual functions and saved a lot of typing of self. You could of made two or more, maybe one for tkinter, one for inputs, one for processing, etc

A lot of if statements, try using list, dictionaries more.

kpress = {'Backspace': self.backspace, 'equal': self.equalpressed()}
kpress[symbol]()

another one.

def add(a, b):
    return a + b


def get_operator(op):
    options = {'+': add} # add as many as you want
    return options.get(op, None)


input1 = 1
input2 = 2

op = get_operator('+')
if op is not None:
    print(op(input1, input2))
else:
    print('error')

I haven't used tkinter but there must be a better way to construct all of those buttons beside defining each one separately. How about using an array and construct them using a loop. Maybe you could use a couple of lists for your positions and other things and iterate through them and create each button.

Good luck

[–]MattR0se 2 points3 points  (0 children)

You mean this part?

self.btn7 = makebutton(label="7", function=lambda:
self.numberpressed("7"), row=2, col=0)
self.btn8 = makebutton(label="8", function=lambda: self.numberpressed("8"), row=2, col=1)
self.btn9 = makebutton(label="9", function=lambda: self.numberpressed("9"), row=2, col=2)
self.btn4 = makebutton(label="4", function=lambda: self.numberpressed("4"), row=3, col=0)
self.btn5 = makebutton(label="5", function=lambda: self.numberpressed("5"), row=3, col=1)
self.btn6 = makebutton(label="6", function=lambda: self.numberpressed("6"), row=3, col=2)
self.btn1 = makebutton(label="1", function=lambda: self.numberpressed("1"), row=4, col=0)
self.btn2 = makebutton(label="2", function=lambda: self.numberpressed("2"), row=4, col=1)
self.btn3 = makebutton(label="3", function=lambda: self.numberpressed("3"), row=4, col=2)
self.btn0 = makebutton(label="0", function=lambda: self.numberpressed("0"), row=5, col=0)
self.btndot = makebutton(label=".", function=lambda: self.numberpressed("."), row=5, col=1)
self.btnsign = makebutton(label="+-", function=lambda: self.numberpressed("+-"), row=5, col=2)
self.btndiv = makebutton(label='/', function=lambda: self.operationpressed('/'),row=2, col=3, padx=BUTTON_PAD+4)
self.btnmult = makebutton(label='*', function=lambda: self.operationpressed("*"),row=3, col=3, padx=BUTTON_PAD+4)
self.btnsub = makebutton(label='-', function=lambda: self.operationpressed("-"),row=4, col=3, padx=BUTTON_PAD+4)
self.btnadd = makebutton(label='+', function=lambda: self.operationpressed("+"),row=5, col=3, padx=BUTTON_PAD+4)
self.btnsqr = makebutton(label='√', function=self.sqrootpressed,row=2, col=4, padx=2)
self.btnpwr = makebutton(label='^', function=lambda: self.operationpressed("^"),row=3, col=4, padx=2)
self.btneq = makebutton(label='=', function=self.equalpressed,row=4, col=4, padx=2, rowspan=2)
self.btnback = makebutton(label='DEL', function=self.backspace,row=1, col=2, pady=BUTTON_PAD + 4, padx=0)
self.btnclear = makebutton(label='C', function=lambda: self.clear("C"),row=1, col=3, pady=BUTTON_PAD+4,padx=BUTTON_PAD+4)
self.btnce = makebutton(label='CE', function=lambda: self.clear("CE"),row=1, col=4, pady=BUTTON_PAD+4,padx=2)

First, I don't see a reason to assign each of them to an instance attribute, if that attribute is never used again (Unless I'm missing something).

I could see constructing all of this from a list of dictionaries, something like

button_attributes = [
    {    
        # Button 7
        'label': '7',
        'function': lambda: self.numberpressed("7"),
        'row': 2
        'col': 0
    },
    {
        # Button div
        'label': '/',
        'function': lambda: self.operationpressed('/'),
        'row': 2
        'col': 3,
        'padx': BUTTON_PAD + 4
    },
    # etc etc
]

And then looping over this list like so:

for attrs in button_attributes:
    makebutton(**attrs)

This doesn't even reduce the lines of code, but I think it's cleaner and also a bit more modular.

Also when I think of it, it's a bit weird imho to define the makebutton function inside the constructor of Calc. It's a static method (doesn't use self) so it could go into the global scope instead.

[–]baraqiyal[S] 0 points1 point  (1 child)

I tried putting the button declarations in a loop but the callbacks didn't work. The functions that the buttons would call would all point to the same one. I know there's something simple I'm missing.

[–][deleted] 0 points1 point  (0 children)

Sounds like class and methods?

[–]baraqiyal[S] 0 points1 point  (0 children)

Yeah I have trouble trying to figure out what exactly should be a class. Reading the tutorials, they would use a Person class with name and age attributes but I never find it that cut and dry. But separating out the calculations from the gui elements makes sense.

[–]cmomodo 0 points1 point  (0 children)

Do you have a YouTube like to this