A Friendly Reminder to Quit Using Method Parameters As Local Variables
Posted in Development
I see this aberration quote often when I refactor code. I noticed Steve McConnell has it down as a rule in Code Complete:
It's dangerous to use parameters passed to a routine as working variables. Use local variables instead.
Take a look at the following contrived example:
public int Sample (int inputVal) { inputVal = inputVal + GetCurrentMultiplier (inputVal); // ... return inputVal; }
The inputVal parameter loses its meaning and purpose as soon as it's used (and abused) as a local variable. Refactoring code this like can quickly become a nightmare. A more politically correct way is to actually use a local variable:
public int Sample (int inputVal) { int workingVal = inputVal; workingVal = workingVal + GetCurrentMultiplier (workingVal); // ... return workingVal; }
Assigning the input value to a working variable emphasizes where the value comes from. It eliminates the possibility that a variable from the parameter list will be modified accidentally.
I'll use McConnell's own excuse here that inputVal and workingVal are
actually terrible names. This is only a drill.
3 comments
Amber
on December 26, 2008
If you have ever picked up a project in the middle of someone else's work you will agree with this statement through and through. The in-class soap box speeches of "One line per statement and comments to define everything" quickly becomes something you pray for. Today's coders and coding can be like reading Chinese...with every other word cut out.
I would love to implement that "NextBigLanguage"
dave
on December 28, 2008
oh no...back to my days crunching Java in "pair programming" courses. I will agree that assigning the value is very important but it really comes down to after a few hundred reworks on your LOC the last thing that you want is repeat these assignments. You want to compile get the green and move on...never to think of that again. That is where I find all of my syntax goes out the window.
@amber - assigning values and keeping a neat code is becoming a little more popular. However, picking up chicken scratch projects and trying to decipher what triangle fits into what round hole is the story of my life.
RichB
on November 30, 2008
Like most code smells, it's only really an issue when you write long methods.
Similarly, the old adage that a function should have a single entry point and a single exit point is also only valid when you write long methods. With short methods, it usually clearer to have multiple exits (return statements).
I'm still waiting for the NextBigLanguage to enforce a maximum LinesPerMethod. That will improve code quality throughout the industry as much as disdain for GOTO did in the 70s/80s.