pyControl / code

pyControl GUI and framework code
https://pycontrol.readthedocs.io
GNU General Public License v3.0
21 stars 20 forks source link

Bug fixes and cleanup #93

Closed alustig3 closed 1 year ago

alustig3 commented 1 year ago
ThomasAkam commented 1 year ago

Thanks @alustig3. The only change I'm not sure about here is the use of Enums rather than strings for some indicator variables, which seems more complicated than the previous implementation. What is the rationale for that?

alustig3 commented 1 year ago

@ThomasAkam the rationale is that it insures at compile-time that the variable/type exists and thus prevents potential mistakes. For instance you could have a conditional statement checking for equality to a misspelled string like having self.state = "Running" instead of self.state = "running". In that situation, the code would run without throwing any errors even though the conditional statement isn't what you intended and will always return False. It is also arguably a little cleaner/more readable.

I don't feel strongly about it and it is probably overkill for this situation considering these variables are only used/accessed a very few times, and within the same file at that.

I've reverted the State(enum) changes.

ThomasAkam commented 1 year ago

Thanks @alustig3, that makes sense. I can see the potential advantage and don't feel stronly either way about using enum here. Will merge this version for now.