The Boy Scout Rule

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
FreddyFreddy
Posts: 176
Joined: Wed Mar 09, 2022 3:15 am
Location: Oz

The Boy Scout Rule

Post by FreddyFreddy »

May I raise the so-called "Boy Scout Rule" (programming version)?

"Always leave the code you're editing a little better than you found it" - Robert C. Martin (Uncle Bob).

Trolling through the codebase and am astounded to find (lots of, just an e.g.)

Code: Select all

if(functionThatReturnsBool()) 
    return true; 
else 
    return false;
and that has been there for years! WTF?

How come nobody thought to tweak that a bit?

Code: Select all

return functionThatReturnsBool();
How to promulgate the idea that everyone who touches the codebase should fix obvious snafus?
ladis
Posts: 75
Joined: Mon Mar 08, 2021 10:53 am
Location: Hohenelbe, Czechia
Contact:

Re: The Boy Scout Rule

Post by ladis »

Coccinelle (Semantic Patch) will help you to turn this idea into reality.
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: The Boy Scout Rule

Post by Kunda1 »

Who wrote that? Lol
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
User avatar
onekk
Veteran
Posts: 6146
Joined: Sat Jan 17, 2015 7:48 am
Contact:

Re: The Boy Scout Rule

Post by onekk »

Or probably the code was left here in this state to eventually expand things when needed.

as example if I would act differently, maybe printing something as error or warning, the code with the if clause is more "extensible" to make such sort of things.

But this is only a guess, probably a "bad guess".

Regards

Carlo D.
GitHub page: https://github.com/onekk/freecad-doc.
- In deep articles on FreeCAD.
- Learning how to model with scripting.
- Various other stuffs.

Blog: https://okkmkblog.wordpress.com/
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: The Boy Scout Rule

Post by openBrain »

FreddyFreddy wrote: Thu Jun 23, 2022 2:00 pm

Code: Select all

if(functionThatReturnsBool()) 
    return true; 
else 
    return false;
and that has been there for years! WTF?
Yes, when I see such a weird thing, I systematically replace with

Code: Select all

return !functionThatReturnsBool() != false?false:true;
I don't like when code can be understood easily by kiddies. :mrgreen: :mrgreen: :mrgreen:
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: The Boy Scout Rule

Post by Kunda1 »

@FreddyFreddy I literally searchedfunctionThatReturnsBool in the source and didn't find it. Did you mean it literally or was that pseudo-code ?

edit: typo
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: The Boy Scout Rule

Post by openBrain »

Kunda1 wrote: Sun Jun 26, 2022 12:10 pm @FreddyFreddy I literally searchedfunctionThatReturnsBool in the source and didn't find it. Did you mean it literally or was that pseudo-code ?

edit: typo
:shock: Yes, that's pseudo code I guess. Would be a really bad name for an actual function. :)
FreddyFreddy
Posts: 176
Joined: Wed Mar 09, 2022 3:15 am
Location: Oz

Re: The Boy Scout Rule

Post by FreddyFreddy »

onekk wrote: Sun Jun 26, 2022 11:48 am Or probably the code was left here in this state to eventually expand things when needed.
I've learnt that is a Really Bad Idea. Causes nothing but pain.
FreddyFreddy
Posts: 176
Joined: Wed Mar 09, 2022 3:15 am
Location: Oz

Re: The Boy Scout Rule

Post by FreddyFreddy »

Kunda1 wrote: Sun Jun 26, 2022 12:10 pm Did you mean it literally or was that pseudo-code ?
Just one of a number in various forms ..

Code: Select all

bool ExpressionParser::isTokenAUnit(const std::string & str)
{
...
    if (status == 0 && token == UNIT)
        return true;
    else
        return false;
}
FreddyFreddy
Posts: 176
Joined: Wed Mar 09, 2022 3:15 am
Location: Oz

Re: The Boy Scout Rule

Post by FreddyFreddy »

openBrain wrote: Sun Jun 26, 2022 11:53 am I don't like when code can be understood easily by kiddies. :mrgreen: :mrgreen: :mrgreen:
A perfect opportunity to add six lines of stupid comments to make sure they never figure it out!
Post Reply