views:

426

answers:

6

I've recently been working with someone else's code and I realized that this individual has a very different philosophy regarding private variables and method parameters than I do. I generally feel that private variables should only be used in a case when:

  1. The variable needs to be stored for recall later.
  2. The data stored in the variable is used globally in the class.
  3. When the variable needs to be globally manipulated (something decidedly different from the need to read the variable by every class method).
  4. When it will make programming substantially easier. (Admittedly vague, but one has to be in many circumstances to avoid painting oneself into a corner).

(I admit, that many of the above are slightly repetative, but they each seem different enough to merit such treatment... )

It just seems that this is the most efficient means of preventing changing a variable by accident. It also seems like following these standards will allow for the eventual manipulation of external references (if the class is eventually modified), thus leaving you with further options in the future. Is this simply a style issue (like one true bracket or Hungarian naming conventions), or do I have justification in this belief? Is there actually a best practice in this case?

edit
I think this needs to be corrected. I used "globally" above where I actually meant, "globally by instance methods" not "globally accessible by anything, anywhere".

edit2
An example was asked for:

class foo
{
    private $_my_private_variable;

    public function __constructor__()
    {
     }

    public function useFoo( $variable )
    {
        // This is the line I am wondering about,
        // there does not seem to be a need for storing it.
        $this->_my_private_variable = $variable; 
        $this->_doSometing();
    }

    private function _doSomething()
    {

        /*
          do something with $this->_my_private_variable.
        */
        // This is the only place _my_private_variable is used.
        echo $this->_my_private_variable;
    }
}

This is the way I would have done it:

class foo
{

    public function __constructor__()
    {
     }

    public function useFoo( $variable )
    {
        $this->_doSometing( $variable );
    }

    private function _doSomething( $passed_variable )
    {
        /*
          do something with the parameter.
        */
        echo $passed_variable;
    }
}
+1  A: 

You should only create variables when and where they are needed, and dispose of them when you are done. If the class doesn't need a class level variable to function, then it just doesn't need one. Creating variables where you don't need them is very bad practice.

David Anderson
+3  A: 

I claim that it isn't a style issue but rather a readability/maintainability issue. One variable should have one use, and one use only. “Recycling” variables for different purposes just because they happen to require the same type doesn't make any sense.

From your description it sounds as if the other person's code you worked on does exactly this, since all other uses are basically covered by your list. Put simply, it uses private member variables to act as temporaries depending on situation. Am I right to assume this? If so, the code is horrible.

The smaller the lexical scope and lifetime of any given variable, the less possiblity of erroneous use and the better for resource disposal.

Konrad Rudolph
Thank you very much. I thought so.
Christopher W. Allen-Poole
+2  A: 

Having a member variable implies that it will be holding state that needs to be held between method calls. If the value doesn't need to live between calls it has no reason to exist outside of the scope of a single call, and thus (if it exists at all) should be a variable within the method itself.

Style is always a hard one, once you develop one you can get stuck in a bit of a rut and it can be difficult to see why what you do may not be the best way.

Alan Mullett
A: 

I'm not sure there is a stated best-practice for using globally scoped variables versus always passing as method parameters. (By "private variables", I'm assuming you mean globally scoped variables.)

Using a globally scoped variable is the only way to implement properties in .NET (even automatic properties ultimately use a globally scoped variable, just not one you have to declare yourself).

There is a line of arguement for always using method parameters because it makes it completely clear where the value is coming from. I don't think it really helps prevent the method from making changes to the underlying value and it can, in my opinion, make things more difficult to read at times.

Scott Dorman
A: 

I would disagree with implementing it for global access or to make programming easier. By exposing these globally without filtering of any kind make it more difficult to determine access in the future.

Michael Glenn
+9  A: 

In general, class members should represent state of the class object.

They are not temporary locations for method parameters (that's what method parameters are for).

Michael Burr