Refactor Parameters To Member Variables

If you find that you're passing the same value, as a parameter, to most methods of a class, then maybe the value should be a member variable of the class.

If the public "entry points" to the class set the member variable, then all the private methods can use the member variable instead of having it as a parameter.

This seems to be more often an issue with "procedural" classes, like CommandObject.


Don't laugh; I just hit a class with one public method and the same half a dozen parameters to every private member in the class. Class also had only one member variable at the time. -- JeffGrigg

Looking at another class, I noticed that every method took a certain object as the first parameter, and that the same instance was passed in each time.

So I made it a member variable, and added a formal constructor/initialization method to set it. -- JeffGrigg

Mmm... I would do this if the object really 'belonged' to the state of the class you're working on. But just adding a member to prevent parameter passing smells like an OO version of global variables. -- IvesAerts

If the same thing is passed to every method of an object, then it seems to me that it probably should be part of the object, or at least strongly related to it. Otherwise why would all the methods need it as a parameter?

My first example was a command object, and the parameters (now member variables) related to the state of the command as it was being executed. My second example was a MockObject for the data layer of a business class, and the parameter was the database connection. "Come on," I said, "maybe the data layer should manage the database connection, rather than making the business layer carry it." And this turned out to be a good choice. -- JeffGrigg


Original example (for RefactorParametersToMemberVariables)

I don't get this. Could you write up an example? I don't see how a method parameter can be directly refactored to an instance variable: You need to give the value of the parameter to the object first, e.g. via a setter.

Example:

Before:

  Class:
  Public Sub a(x, y, z)
     Call b(x, y, z, 12, "This")
     Call c(x, y, z, #12/11/2002#, 6.4)
  End Sub
  Private Sub b(x, y, z, i, s)
     '...
  End Sub
  Private Sub c(x, y, z, d, f)
     '...
  End Sub
After:
  Class:
  Private mx, my, mz
  Public Sub a(x, y, z)
     mx = x; my = y; mz = z
     Call b(12, "This")
     Call c(#12/11/2002#, 6.4)
  End Sub
  Private Sub b(i, s)
     '...  (now uses 'mx' instead of 'x', etc...)
  End Sub
  Private Sub c(d, f)
     '...
  End Sub


Modified example (of IntroduceParameterObject, and MoveMethod)

What about using IntroduceParameterObject instead? -- anon

True, I'd be passing one thing instead of several, but I'd still be passing it to just about everything. -- JeffGrigg

In the next step you'd use MoveMethod : the destination is the class created from the "cluster" of parameters, the methods to move are all the methods to which you are passing this cluster. All of a sudden you have a new class with actual behaviour... The heuristic for IntroduceParameterObject is that parameters that go together often "hide" a true class.

To rework your example (I don't write VB so please bear with me if this isn't syntactically correct) :

Before:

  Class 1:
  Public Sub a(x, y, z)
     Call b(x, y, z, 12, "This")
     Call c(x, y, z, #12/11/2002#, 6.4)
  End Sub
  Private Sub b(x, y, z, i, s)
     '...
  End Sub
  Private Sub c(x, y, z, d, f)
     '...
  End Sub

After IntroduceParameterObject :
  Class XYZ:
  Public mx, my, mz

Class 1: Public Sub a(xyz As XYZ) Call b(xyz, 12, "This") Call c(xyz, #12/11/2002#, 6.4) End Sub Private Sub b(xyz As XYZ, i, s) '... (now uses 'xyz.mx' instead of 'x', etc...) End Sub Private Sub c(xyz As XYZ, d, f) '... End Sub

After MoveMethod :
  Class XYZ:
  Private mx, my, mz
  Public Sub b(i, s)
     '...  (now uses 'mx' instead of 'xyz.mx' or 'x'...)
  End Sub
  Public Sub c(d, f)
     '...
  End Sub

Class 1: Public Sub a(xyz) Call xyz.b(12, "This") Call xyz.c(#12/11/2002#, 6.4) End Sub

Grok ?


Maybe...

Before:

  Class 1:
  Public Sub a(x, y, z)
     Call b(x, y, z, 12, "This")
     Call c(x, y, z, #12/11/2002#, 6.4)
  End Sub
  Private Sub b(x, y, z, i, s)
     '...
  End Sub
  Private Sub c(x, y, z, d, f)
     '...
  End Sub

After IntroduceParameterObject :
  Class XYZ:
  Public mx, my, mz
  Public Sub Init(x, y, z)
     mx = x; my = y; mz = z
  End Sub

Class 1: Public Sub a(x, y, z) Dim xyz As New XYZ Call xyz.Init(x, y, z) Call b(xyz, 12, "This") Call c(xyz, #12/11/2002#, 6.4) End Sub Private Sub b(xyz As XYZ, i, s) '... (now uses 'xyz.mx' instead of 'x', etc...) End Sub Private Sub c(xyz As XYZ, d, f) '... End Sub

After MoveMethod :
  Class XYZ:
  Private mx, my, mz
  Public Sub Init(x, y, z)
     mx = x; my = y; mz = z
  End Sub
  Public Sub b(i, s)
     '...  (now uses 'mx' instead of 'xyz.mx' or 'x'...)
  End Sub
  Public Sub c(d, f)
     '...
  End Sub

Class 1: Public Sub a(xyz) Dim xyz As New XYZ Call xyz.Init(x, y, z) Call xyz.b(12, "This") Call xyz.c(#12/11/2002#, 6.4) End Sub

Issue: If b() or c() already use any member variables of Class 1, then they'll have to be passed in the Init, if static across calls, or as parameters, if they can change.

Also, at this point, why not just move 'a()' to class XYZ, and eliminate 'Class 1'?

Presumably because Class 1 also had other methods that did not use x, y, and z.


[Opposite of RefactorScopedVariableToParameter]

[CategoryRefactoring, RefactoringLanguage]


EditText of this page (last edited January 12, 2002) or FindPage with title or text search