Rover656 / Ngine

A C++ 17 library for producing 2D games.
https://www.nerdthings.dev/ngine
Apache License 2.0
2 stars 2 forks source link

Refactor 17 lines occurring 3 times in TilesetCanvas.cpp #16

Closed Rover656 closed 4 years ago

Rover656 commented 5 years ago

I've selected for refactoring 17 lines of code which are duplicated in 1 file(s) (1, 2, 3). Addressing this will make our codebase more maintainable and improve Better Code Hub's Write Code Once guideline rating! 👍

Here's the gist of this guideline:

You can find more info about this guideline in Building Maintainable Software. 📖


ℹ️ To know how many other refactoring candidates need addressing to get a guideline compliant, select some by clicking on the 🔲 next to them. The risk profile below the candidates signals (✅) when it's enough! 🏁


Good luck and happy coding! :shipit: :sparkles: :100:

UltimaBGD commented 5 years ago

Are you still looking for assistance on this? If so, I can take a look at it.

In addition, do you have any specific requests on how to handle the for loops?

Rover656 commented 5 years ago

Hi there, first of all, thanks for your interest. Feel free to have a go at any of these issues. All of these methods could probably be rewritten as one single method that does everything. If you have any better ideas, feel free to create a PR! :smile:

UltimaBGD commented 5 years ago

We can extract the duplicated code and the for loops to one method that each of the 3 other methods would call with a flag telling the merged function what for loop to run. The for loop statements would still technically be duplicated in that instance, but it may be better to have those lines repeated than have a check in the for loops for what code to run.

UltimaBGD commented 5 years ago

The validation code alone could easily be extracted to a smaller function, but that leaves the creation of the shape vector duplicated across each, as well as the creation of sX sY eX eY as pointers to pass in for validation. This would be a small scale fix that still fixes the duplication of 10 lines each time.

Rover656 commented 4 years ago

Closing this as the class was removed for rewriting in the develop branch.