Java Instance Variables vs Local Variables

I haven't seen anyone discuss this so I'll throw in more food for thought. The short answer/advice is don't use instance variables over local variables just because you think they are easier to return values. You are going to make working with your code very very hard if you don't use local variables and instance variables appropriately. You will produce some serious bugs that are really hard to track down. If you want to understand what I mean by serious bugs, and what that might look like read on.

Let's try and use only instance variables as you suggest to write to functions. I'll create a very simple class:

public class BadIdea {
   public Enum Color { GREEN, RED, BLUE, PURPLE };
   public Color[] map = new Colors[] { 
        Color.GREEN, 
        Color.GREEN, 
        Color.RED, 
        Color.BLUE, 
        Color.PURPLE,
        Color.RED,
        Color.PURPLE };

   List<Integer> indexes = new ArrayList<Integer>();
   public int counter = 0;
   public int index = 0;

   public void findColor( Color value ) {
      indexes.clear();
      for( index = 0; index < map.length; index++ ) {
         if( map[index] == value ) {
            indexes.add( index );
            counter++;
         }
      }
   }

   public void findOppositeColors( Color value ) {
      indexes.clear();
      for( index = 0; i < index < map.length; index++ ) {
         if( map[index] != value ) {
            indexes.add( index );
            counter++;
         }
      }
   }
}

This is a silly program I know, but we can use it to illustrate the concept that using instance variables for things like this is a tremendously bad idea. The biggest thing you'll find is that those methods use all of the instance variables we have. And it modifies indexes, counter, and index every time they are called. The first problem you'll find is that calling those methods one after the other can modify the answers from prior runs. So for example, if you wrote the following code:

BadIdea idea = new BadIdea();
idea.findColor( Color.RED );
idea.findColor( Color.GREEN );  // whoops we just lost the results from finding all Color.RED

Since findColor uses instance variables to track returned values we can only return one result at a time. Let's try and save off a reference to those results before we call it again:

BadIdea idea = new BadIdea();
idea.findColor( Color.RED );
List<Integer> redPositions = idea.indexes;
int redCount = idea.counter;
idea.findColor( Color.GREEN );  // this causes red positions to be lost! (i.e. idea.indexes.clear()
List<Integer> greenPositions = idea.indexes;
int greenCount = idea.counter;

In this second example we saved the red positions on the 3rd line, but same thing happened!?Why did we lose them?! Because idea.indexes was cleared instead of allocated so there can only be one answer used at a time. You have to completely finish using that result before calling it again. Once you call a method again the results are cleared and you lose everything. In order to fix this you'll have to allocate a new result each time so red and green answers are separate. So let's clone our answers to create new copies of things:

BadIdea idea = new BadIdea();
idea.findColor( Color.RED );
List<Integer> redPositions = idea.indexes.clone();
int redCount = idea.counter;
idea.findColor( Color.GREEN );
List<Integer> greenPositions = idea.indexes.clone();
int greenCount = idea.counter;

Ok finally we have two separate results. The results of red and green are now separate. But, we had to know a lot about how BadIdea operated internally before the program worked didn't we? We need to remember to clone the returns every time we called it to safely make sure our results didn't get clobbered. Why is the caller forced to remember these details? Wouldn't it be easier if we didn't have to do that?

Also notice that the caller has to use local variables to remember the results so while you didn't use local variables in the methods of BadIdea the caller has to use them to remember results. So what did you really accomplish? You really just moved the problem to the caller forcing them to do more. And the work you pushed onto the caller is not an easy rule to follow because there are some many exceptions to the rule.

Now let's try doing that with two different methods. Notice how I've been "smart" and I reused those same instance variables to "save memory" and kept the code compact. ;-)

BadIdea idea = new BadIdea();
idea.findColor( Color.RED );
List<Integer> redPositions = idea.indexes;
int redCount = idea.counter;
idea.findOppositeColors( Color.RED );  // this causes red positions to be lost again!!
List<Integer> greenPositions = idea.indexes;
int greenCount = idea.counter;

Same thing happened! Damn but I was being so "smart" and saving memory and the code uses less resources!!! This is the real peril of using instance variables like this is calling methods is order dependent now. If I change the order of the method calls the results are different even though I haven't really changed the underlying state of BadIdea. I didn't change the contents of the map. Why does the program yield different results when I call the methods in different order?

idea.findColor( Color.RED )
idea.findOppositeColors( Color.RED )

Produces a different result than if I swapped those two methods:

idea.findOppositeColors( Color.RED )
idea.findColor( Color.RED )

These types of errors are really hard to track down especially when those lines aren't right next to each other. You can completely break your program by just adding a new call in anywhere between those two lines and get wildly different results. Sure when we're dealing with small number of lines it's easy to spot errors. But, in a larger program you can waste days trying to reproduce them even though the data in the program hasn't changed.

And this only looks at single threaded problems. If BadIdea was being used in a multi-threaded situation the errors can get really bizarre. What happens if findColors() and findOppositeColors() is called at the same time? Crash, all your hair falls out, Death, space and time collapse into a singularity and the universe is swallows up? Probably at least two of those. Threads are probably above your head now, but hopefully we can steer you away from doing bad things now so when you do get to threads those bad practices don't cause you real heartache.

Did you notice how careful you had to be when calling the methods? They overwrote each other, they shared memory possibly randomly, you had to remember the details of how it worked on the inside to make it work on the outside, changing the order in which things were called produce very big changes in the next lines down, and it only could only work in a single thread situation. Doing things like this will produce really brittle code that seems to fall apart whenever you touch it. These practices I showed contributed directly to the code being brittle.

While this might look like encapsulation it is the exact opposite because the technical details of how you wrote it have to be known to the caller. The caller has to write their code in a very particular way to make their code work, and they can't do it without knowing about the technical details of your code. This is often called a Leaky Abstraction because the class is suppose to hide the technical details behind an abstraction/interface, but the technical details leak out forcing the caller to change their behavior. Every solution has some degree of leaky-ness, but using any of the above techniques like these guarantees no matter what problem you are trying to solve it will be terribly leaky if you apply them. So let's look at the GoodIdea now.

Let's rewrite using local variables:

 public class GoodIdea {
   ...

   public List<Integer> findColor( Color value ) {
      List<Integer> results = new ArrayList<Integer>();
      for( int i = 0; i < map.length; i++ ) {
         if( map[index] == value ) {
            results.add( i );
         }
      }
      return results;
   }

   public List<Integer> findOppositeColors( Color value ) {
      List<Integer> results = new ArrayList<Integer>();
      for( int i = 0; i < map.length; i++ ) {
         if( map[index] != value ) {
            results.add( i );
         }
      }
      return results;
   }
 }

This fixes every problem we discussed above. I know I'm not keeping track of counter or returning it, but if I did I can create a new class and return that instead of List. Sometimes I use the following object to return multiple results quickly:

public class Pair<K,T> {
    public K first;
    public T second;

    public Pair( K first, T second ) {
       this.first = first;
       this.second = second;
    }
}

Long answer, but a very important topic.

Use instance variables when it's a core concept of your class. If you're iterating, recursing or doing some processing, then use local variables.

When you need to use two (or more) variables in the same places, it's time to create a new class with those attributes (and appropriate means to set them). This will make your code cleaner and help you think about problems (each class is a new term in your vocabulary).

One variable may be made a class when it is a core concept. For example real-world identifiers: these could be represented as Strings, but often, if you encapsulate them into their own object they suddenly start "attracting" functionality (validation, association to other objects etc)

Also (not entirely related) is object consistency - an object is able to ensure that it's state makes sense. Setting one property may alter another. It also makes it far easier to alter your program to be thread-safe later (if required).

Local variables internal to methods are always prefered, since you want to keep each variable's scope as small as possible. But if more than one method needs to access a variable, then it's going to have to be an instance variable.

Local variables are more like intermediate values used to reach a result or compute something on the fly. Instance variables are more like attributes of a class, like your age or name.

Short story: if and only if a variable needs to be accessed by more than one method(or outside of the class) create it as an instance variables. If you need it only locally, in a single method, it has to be a local variable. Instance variables are more costly than local variables.
Keep in mind: instance variables are initialized to default values while local variables are not.

Declare variables to be scoped as narrowly as possible. Declare local variables first. If this isn't sufficient, use instance variables. If this isn't sufficient, use class (static) variables.

I you need to return more than one value return a composite structure, like an array or an object.

The easy way: if the variable must be shared by more than one method, use instance variable, otherwise use local variable.

However, the good practice is to use as more local variables as possible. Why? For your simple project with only one class, there is no difference. For a project that includes a lot of classes, there is big difference. The instance variable indicates the state of your class. The more instance variables in your class, the more states this class can have and then, the more complex this class is, the hard the class is maintained or the more error prone your project might be. So the good practice is to use as more local variable as possible to keep the state of the class as simple as possible.

Try to think about your problem in terms of objects. Each class represents a different type of object. Instance variables are the pieces of data that a class needs to remember in order to work, either with itself or with other objects. Local variables should just be used intermediate calculations, data that you don't need to save once you leave the method.

Try not to return more than one value from your methods in first place. If you can't, and in some cases you really can't, then I would recommend encapsulating that in a class. Just in last case I would recommend changing another variable inside your class (an instance variable). The problem with the instance variables approach is that it increases side effects - for example, you call method A in your program and it modifies some instance(s) variable(s). Over time, that leads to increased complexity in your code and maintenance becomes harder and harder.

When I have to use instance variables, I try to make then final and initialize then in the class constructors, so side effects are minimized. This programming style (minimizing the state changes in your application) should lead to better code that is easier to maintain.

Everyone has the right basic idea... but the wrong answer if you want low maintenance.

Many java classes have instance variables like, OneBigHugeDetailedFactoryOrObjectThatDoesThisOrThat bhtt = null;

1. Putting that reference in methods is a huge REUSE downer!
2. Refactoring is also held back, since local variables passed via parameters from method to method are cumbersome.
3. Avoiding complexity can be helped by removing anything not necessary.

Caveat: If you KNOW you will only use this class once in a method, of course do it locally. If you have little "i" and "n" integers, etc. Of course do it locally!

But if you are in the middle of designing a class... or you are thinking of maintenance, you will often find fewer lines of cluttered code by using instance variables.

If you want to minimize complexity in an algorithm, you need to see as much as possible about the algorithm in as little area as possible.

It should be like this:
    bhtt  = new OneBigHugeDetailedFactoryOrObjectThatDoesThisOrThat(foo, bar, true);
    bhtt.doThis(233, paramA, paramB, paramC);
    bhtt.add(something);


NOT THIS!
    OneBigHugeDetailedFactoryOrObjectThatDoesThisOrThat bhtt = new OneBigHugeDetailedFactoryOrObjectThatDoesThisOrThat(foo, bar, true);
    bhtt.doThis(233, paramA, paramB, paramC);
    bhtt.add(something);

This way you can add or remove methods to operate on this object and most easily end up with the best class organization. Using more instance variables initially, in design, around complex code or with large name classes allow one to minimize complexity. When you are done refactoring, you can always use your favorite code checking (IDE/PMD) to remove unneeded instance variables when the class is stable. Usually a better designed class will end up with more smaller methods, than fewer larger methods. This will be frustrating if you have to keep instantiating or passing instance parameters around.

Many experienced programmers mess this up because they get obsessed about so called "best" form and "best practice", and they end up losing flexibility. They create a mess for the next programmer who must deal with a poorly designed class. The best thing to do is to keep your class organized at the method level by getting rid of baggage around your object manipulation business logic (get rid of verbose local class names... and REUSE the instance within various methods).

And if you have too many instance variables to keep track of, your class may be getting too big.

Sometimes "Best Practice" is the wrong way to go.

The intuition of the questioner is more "Right" than the "conventional answer"!

My 2 cents.