Coding style: const QString foo = foo();

[Albert Astals Cid]

El dilluns, 28 d’octubre del 2024, a les 10:59:47 (Hora estàndard del Centre 
d’Europa), Sune Vuorela va escriure:
> Hi peoples
> 
> Quite some repositories have enabled cppcheck with a ruleset that I more
> or less have composed so far. I do think it provides valuable feedback,
> especially if we can get the rules groomed sufficiently and fix the
> remaining points.
> 
> It at least have helped me with quite some bugs over time.
> 
> There is though one pattern that cppcheck complains about that I'm a bit
> unsure if we should ask cppcheck to not complain about or if we should
> fix the instances.
> 
> | Minor - Local variable 'foregroundColor' shadows outer function
> 
> Basically, various versions of people doing
> 
> | const QString foo = foo();
> 
> inside their functions. Is this a issue we should suppress at cppcheck
> level, or one where we should rename the variable ?
> 
> There are a bit of pros and cons.
> 
> The biggest argument for adapting cppcheck settings is that it is
> unlikely that anyone will actually be confused by this kind of
> shadowing, and most IDE's will help you anyways.
> Another big argument for doing it as a cppcheck setting is that it is
> normally 'the good name' for the thing in question, so the variable will
> be less good. Either people start adding tails_ to the variable or weird
> abbrvs.
> 
> The biggest argument for adapting the code is that it makes things more
> readable for people without a IDE and for simpler search/grep in source
> code.
> 
> 
> (by IDE I also includue various language-server additions to editors)
> 
> Does anyone have opinions here?

My vote goes for rename the variable, there's 2 reasons i can think why you do 

const QString foo = foo();

(there's probably many more, i spent 5 seconds thinking)

Reason 1: You're caching it because foo is expensive, rename the variable to 
cachedFoo

Reason 2: You're remembering it because you're going to change it and then 
later want to compare against it, rename it to oldFoo

Cheers,
  Albert

> 
> /Sune