CytopiaTeam / Cytopia

:deciduous_tree::house_with_garden::office::evergreen_tree: A city building simulation game
https://www.cytopia.net
GNU General Public License v3.0
1.97k stars 105 forks source link

uimanager: Prefer standard algorithm - `std::none_of` #1014

Closed tcoyvwac closed 2 years ago

tcoyvwac commented 2 years ago

Minor edits to prefer standard library function std::none_of over std::find.

lizzyd710 commented 2 years ago

Dang, now the Windows check is failing, but it looks like another thing with Conan...

lizzyd710 commented 2 years ago

Good work/catch. I'm curious, does this improve the performance, and if so how much? If none_of is better than find for stuff like this, would you mind converting in other files?

lizzyd710 commented 2 years ago

Can you update/rebase this? We pushed a fix to master that took care of the failing checks.

tcoyvwac commented 2 years ago

Hi @lizzyd710,

std::none_of was introduced 11 years ago in the C++11 standards. The abstraction kindly helps to explain to developers what the code does in a much clearer way.

For example, with std::none_of, it is assumed that: one wants a bool result that no element exists of x in an array / vector. This is not the case with using the function std::find. With std::find, its task is: to retrieve an iterator (to modify or not) of element x located in an array / vector. To use the result of std::find, one must do an error-bounds check separately, while for std::none_of, it does this check within (itself).

This means that the previous C++03ish code here does the same thing as the std::none_of code.

Bjarne Stroustrup in an interview in 2004:

Bjarne Stroustrup: A high level of abstraction is good, not just in C++, but in general. We want to deal with problems at the level we are thinking about those problems. When we do that, we have no gap between the way we understand problems and the way we implement their solutions. We can understand the next guy's code. We don't have to be the compiler. [...] I believe raising the level of abstraction is fundamental in all practical intellectual endeavors. I don't consider that a controversial statement, but people sometimes consider it controversial because they think code at a higher level abstraction is necessarily less efficient.

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

lizzyd710 commented 2 years ago

Are you on Discord? I really appreciate your contributions and would love to talk more about it/if you want to help more on there. If not that’s fine we can talk on here!

tcoyvwac commented 2 years ago

Once again / As always, thanks for merging @lizzyd710, Grateful every time! I prefer through (GitHub) PRs to communicate sorry! Happy to contribute changes to your project no problem! :smile_cat:

lizzyd710 commented 2 years ago

No need to apologize for your preference! Would you be interested in being added to @CytopiaTeam/developers ?

tcoyvwac commented 2 years ago

Hi @lizzyd710 :smilecat: , Humm, maybe there is a blocker in merge requests being processed at the moment. Would it help you & the project, if being added to your CytopiaTeam/developers, and helping the response time to other GItHub merge requests? (I like to refactor your project Cytopia casually in my down-time so for now, and after seeing your previous comment, it could help ya having extra staff)._ :cat2:

lizzyd710 commented 2 years ago

Yeah, some of our devs have stepped back from the project (which is totally fine, this is just for fun!) and I don't want to bypass requirements (although the ones we have open right now I think are stale) so that would be helpful! Also appreciate the refactoring help!