public void foo(String bar)
{
bar = bar + "/"; // FP
}
The rule reminds me not to ignore input parameters (and caught exceptions and foreach arguments). That’s fine. The parameter is not ignored here, though. It is used to derive a new value and assign it to the variable.
Yes, you could argue, that you should never assign to an input parameter. But that’s not in the domain of this rule, and should not be.
My actual code look a little bit different. The FP line doesn’t to a simple concatenation, but calls a method, that possibly trims the string and returns it. But the simplified code should do it.
In my opinion, the main problem with this rule is its text content.
While the title suggests one thing, the description and Noncompliant code example show other problems.
I will not create a ticket for fixing this, as we plan to improve and update all rule descriptions.
yes, I agree. The title and description text deverge a little. So what do you see as the problem here? The title? Or the description text?
I think, both can be misunderstood. What do you want the rule to suggest? Never assign to arguments? Always treat them as final?
The (current) description text says, that the value should at least be read once before being assigned. In my example it is read before assigned. So the rule should not hit, if the description is correct.
I think, these are two related, but yet separate rules.
Never ever assign to arguments!
Never assign to arguments before it is read at least once.
I think, both rules have a right to exist. I personally would not want to activate (1.), but (2.) would be good.
And I would make a difference between method parameters and caught exceptions and especially foreach variables. Writing to the latter is definitely a bad idea. Writing to caucht exception variables is quite unlikely. But there are many good scenarios to write to method parameters, if you don’t want to follow a strict (1.).
Hope, this helps a little to make something better out of these rules, that are currently condensed into one.