LucasPilla / Sorting-Algorithms-Visualizer

Program made with Python and Pygame module for visualizing sorting algorithms
MIT License
426 stars 165 forks source link

turned sorting functions into generators #125

Closed mugulmd closed 3 years ago

mugulmd commented 3 years ago

Sorting functions have become generators by simply replacing calls to handleDrawing by yield statements : this way their execution is still paused during the delay set by the user, the algorithm package is independent of the rest of the code, and there's only one event loop which is kept in the main function. We use an iterator in the main function to capture the results of the yield statements, which are then used for drawing.

GlacialBlaze commented 3 years ago

@mugulmd

The use of generators is a good move.

However, I found the following:

Please check the code again.

mugulmd commented 3 years ago

Okay I think it should work now :

GlacialBlaze commented 3 years ago

Good progress.

However, I found the following:

Perhaps a little adjustment somewhere?

mugulmd commented 3 years ago

Done :)

GlacialBlaze commented 3 years ago

The visualizer seems to be free from error now - good job.

However, let us explore just a little bit on the button mechanism. This is not an error but I notice that the button is doing too much work, like having two states, and each state is constantly updated, and the switching off-and-on is rather complicated.

Can the way the button works be simplified? Perhaps a little change in the main loop or the update of the ButtonBox?

mugulmd commented 3 years ago

I agree with you but I don't see any "unnecessary" code :/ I think the main reason is that a 2 states button is a quite complex object, that does various actions and gets updated in various ways depending on the context, so you have to check the different configurations of this context. Moreover you have to code the different actions outside of the ButtonBox class because they are specific to this play-stop button. It would probably be clearer and more efficient to create 2 buttons that only have 1 state (1 for play and 1 for stop), each one being able to execute only 1 action, and switching between them when they are clicked (which is what I did when writing the gui package).

As for code clarity we could just put the different actions executed when clicking the button inside separated functions and call these functions in the main loop (and maybe add some more comments).

However if you have a way to simplify things that I didn't see I'd be happy to hear it of course :)

GlacialBlaze commented 3 years ago

I see.

I'm interested in the '2 buttons thing (1 for play, the other for stop)' which you said you had implemented before.

Would it add way more lines of code? Is it too much of work to implement that?

mugulmd commented 3 years ago

I don't think so, I can try later today :)

mugulmd commented 3 years ago

I think it works just fine and the code is a bit more elegant indeed :)

GlacialBlaze commented 3 years ago

The play-stop mechanism looks simpler and better - good work.

Now, there's this thing I noticed: variables.py. Can those global variables be put in display.py to save an extra file and make consistent the variable directory (like all display.... rather than having an extra variable....) in main.py?

mugulmd commented 3 years ago

Indeed this new file wasn't very useful ^^ I removed it and updated the code :)

GlacialBlaze commented 3 years ago

The code appears better now.

Our friend @rx112358 has created a vertical scrollbar on his side. I merged his pull request. @mugulmd Could you please update your code? The difference should be only in display.py, line 95 - 120 of the master branch. Perhaps try Resolve conflicts below?

Sorry for the extra hassle.

mugulmd commented 3 years ago

Ok sorry for all the useless commits I was trying to solve the conflict locally but I found out you can do it directly on Github's interface and it's a lot easier ^^

GlacialBlaze commented 3 years ago

@mugulmd It's OK. Sorry to have you update it spontaneously. Thanks.

@LucasPilla Sorting functions no longer need to import handleDrawing as they are generators that simply yield results. Most of the actions are now compactly handled in the master loop in main.py. And some other minor improvements. 😊