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
