search

moodlehq / moodle-cs

Moodle Coding Style
https://github.com/moodlehq/moodle-cs
GNU General Public License v3.0
18 stars 16 forks source link

Who turned on "Empty IF statement detected" and why? #98

Closed timhunt closed 9 months ago

timhunt commented 9 months ago

This code https://github.com/moodle/moodle/blob/328b48ebc592a8f0a551aacc6e2e447a50a50c90/mod/quiz/review.php#L196 has been there for years, and is the clearest way to express the correct logic. Who though that should start generating codechecker warnings? (I never saw a policy issue about this.)

jrchamp commented 9 months ago

It looks like you did it in 2011: https://github.com/moodlehq/moodle-cs/blame/39d8a28172f63c9cec14eb7491d9c1db619784b3/phpcs/moodle/ruleset.xml#L28

timhunt commented 9 months ago

In that case, perhaps upstream have changed the bahviour of that sniff recently, so they no longer count a comment as sufficient. (I fell it is reasonable to insist on a comment to explain when this patten is used.)

stronk7 commented 9 months ago

I've not looked if/when that behaviour has changed, but I'm pretty sure that it has been there since long, long ago.

Because I used to "abuse" those empty conditions a lot and now, instead, I always use conditions leading to real "food" (code) (and, personally, I think it's good practice).

Not defending anything, just saying/sharing that I think - 80% sure - it's not a "new" thing.

Ciao :-)

timhunt commented 9 months ago

OK. Whatever.

There are times when I think the style leads to much cleaner code. I guess I can jsut ignore this rule when it complains.