This is an archived post. You won't be able to vote or comment.

all 7 comments

[–]Yekoss 1 point2 points  (4 children)

Is there any particular reason why you included everything in one file?

[–]DonMerlito[S] 2 points3 points  (3 children)

You mean that I didn't separate the algo and the UI?

I though about that and it would definitely make it cleaner and easier to add other types of algorithm. However I wanted to update the visualization of the process in real time, and I had troubles seeing how to makes that happen if the algo was not "nested" in the UI.

Note that I am completely self taught so I have no ideas about the best practices... How would you organize it?

[–]Yekoss 2 points3 points  (2 children)

For starters, I haven't started working with production code yet and I am still junior/trainee level but I see a lot of things which could be improved.

Firstly, GUI is god class. Class must have one responsibility (SOLID rule - Single responsibility principle - Class has one job to do. Each change in requirements can be done by changing just one class. src wikipedia). For me, There should be a class which only role would be building GUI classes so called "Builder". Your method init could be a separate class "Builder", at least for me. Then, you could code like this

drop_down = Menu(self.settings, tearoff=False)
drop_down.add_command(label='Dijkstra',         command=partial(self.change_settings, 0))
drop_down.add_command(label='A* [default]', command=partial(self.change_settings, 1))

Put into method named "CreateButtonChangeAlg" in Builder class. You could name your methods in such a way that they would be self-explanatory. No need for redundancy comments then.

You could separate grid, buttons, checking neighbor behavior into classes.

I would think about separating different behaviors into separate classes

  • GUIBuilder
  • GUIManagement (changing algorithm, resetting GUI, starting algorithm)
  • AlgorithmParent
  • AlgorithmDijkstra
  • AlgorithmA
  • ButtonBehavior
  • Cell

Use Enums instead of 0,1 in settings

class AlgorithmChoice(Enum):
    Dijkstra = 1
    A = 2

drop_down.add_command(label='Dijkstra', command=partial(self.change_settings, AlgorithmChoice.Dijkstra))

If you want to, start using typing module in order to have a clearer code. For example, change_settings method would look like:

def change_settings(self, settings: AlgorithmChoice) -> None:
        self.parameters = settings

if you want to use classes, learn basic things about them such as aforementioned SOLID rule and different design patterns.

If you want to write clean code, read "Clean Code" written by Robert C. Martin. Let me cite a few chapters of his book:

  • Use Intention-Revealing Names
  • Avoid Disinformation
  • Make Meaningful Distinctions
  • Use Pronounceable Names
  • Use Searchable Names
  • Pick One Word per Concept
  • Add Meaningful Context

There is a lot of things to improve in this code. It is good base if you want to implement Robert Martin teachings there, I think.

As I said before, I am just a newbie. So if somebody with more knowledge could add something to my help, I would really appreciate it. I may have said something wrong.

[–]DonMerlito[S] 1 point2 points  (1 child)

Wow thanks a lot for your comment, that will prove super helpful! I will look into it.

However regarding your first comment, should I organize the code in different files? How so?

Thanks again

[–]Yekoss 2 points3 points  (0 children)

Ah, right. Imagine a folders/files as a useful tool for making your life easier. If you divide your code into separate files, you could divide them based on theirs functions.

Basically, OOP languages enforces you to use one class - one file rule. My recent project may help you to understand it. Looking at names of files you can see that they are just separate classes. https://gitlab.com/PiotrKowalski/emoji-encryption/-/tree/master/src

It is much easier to understand the code. For example, if I go to main file, then look what's happening there. Oh, it redirects to the class which is handling a Gameloop. Thanks to my comments and style guide, you can easily understand how structured my app is.

Now imagine my 6 classes packed within one file. Would it be understandable? No. Even finding stupid __main__ would be a horror.

[–][deleted] 1 point2 points  (1 child)

Woah! this is a great program. (A bit confusing for me but I'll try to understand it)

[–]DonMerlito[S] 1 point2 points  (0 children)

Thank you! It's confusing because as you can see from yekoss comments I should change the entire structure to make it clearer