search

Loobinex / keeperfx-unofficial

KeeperFX - Unofficial release
60 stars 7 forks source link

Automated code clean up #119

Closed Loobinex closed 4 years ago

Loobinex commented 4 years ago

Moves variables to the scope where they actually belong.

Loobinex commented 4 years ago

@spartahawk and @Trass3r do you agree with Trass3r that the code is 'cleaner' like this, or do you prefer the original method?

spartahawk commented 4 years ago

Yes, more glance-able and follows clean code principles better (adding curly braces for all the one-liner next line if statements would be good too).

There's no way I'm going to look through all of it to make sure it never makes a change that might break something though. Be sure everything works as expected. Have you checked that game saves work, or whatever it was?

Loobinex commented 4 years ago

I get that. And no, I've yet to test it. But that's the thing 'everything working as expected' is a bit much too. There's no automated testing on this thing.

spartahawk commented 4 years ago

Having said that about cleaner code, I've never been in such a hurry with this code base that I needed those extra seconds to look a few lines back to check a variable that might have been declared in too general of a scope. If the automated tool that did this were already well-versedvetted, I'd say, sure, do it. But at the risk of changes we can't be sure of with total confidence, combined with a lack of current code being any real problem, I don't think this is overall better, necessarily