Why don't use static variable to share object for the subclass?

Question

My question is a bit long. I am learning abstract factory pattern.

I have a abstract class for the abstract factory. I want to share the "resources" needed by the concrete factories. So I simply make the variable inside AbstractFactory as static

public class AbstractFactory{
    private static Vector vector = new Vector();

    protected Vector void getVector() {
        return vector;
    }

    protected void setVector(Vector v){
        this.vector = v;
    }

    public abstract Circle createCircle();
}

And its Subclass will look like:

public class ConcreteFactory extends AbstractFactory{
    public ConcreteFactory(){
        super();
    }

    public Circle createCircle(){
        Circle circle = new Circle();
        getVector().add(circle);
        return circle;
    }
}

However, my teacher said that I should not use the static object instance because static variables are often used for some constants.

Therefore, I use instance variable instance instead of static variable for the Vector , and I pass the vector from outside when I instantiate the concrete factory.

So the new design of my classes will look like:

public class AbstractFactory{
    private Vector vector;

    protected Vector void getVector() {
        return vector;
    }

    protected void setVector(Vector v){
        this.vector = v;
    }

    public abstract Circle createCircle();
}

public class ConcreteFactory extends AbstractFactory{
    public ConcreteFactory(Vector v){
        super();
        setVector(v);
    }

    public Circle createCircle(){
        Circle circle = new Circle();
        getVector().add(circle);
        return circle;
    }
}

**

My question is : why I should not use the static variable to share object?

**

It will be easier to share resources among the concrete factories without passing in the Vector when I create an instance of concrete factories.

Answer 1

The fact that static final variables are sometimes used as constants should not prevent you from using the same mechanism wherever it servers the best (logger is the only other example I can give).

It is always good though, when there is no implicit component coupling to the outer world. If you define vector as a static variable you leave user no possibility to make your factories be context based and independent from one other. If you make the vector an argument of the factory then it is up to the factory creator (typically Spring context loader nowdays) to define which factories share the vector and which do not.

Another reason to pass vector as an argument in your case is related to the unit testing aspect. If your class is designed to take vector from the outside you can mock it and test your class more thoroughly.

Answer 2

Sometimes what makes your life easier today will make it much, much more difficult down the road.

Some examples: You don't always know the environment where your class will be used. Maybe different instances of your factories will wind up being loaded by different class loaders (this happens often in web applications). You could wind up with completely unpredictable behavior using a static instance variable that way. Static variables are almost always a bad idea.

That said, I think that what you really want to do in your class is this:

public class AbstractFactory{
    private final Vector vector;

    protected AbstractFactory(Vector vector){
        this.vector = vector;
    }

    protected Vector void getVector() {
        return vector;
    }

    public abstract Circle createCircle();
}

and

public class ConcreteFactory extends AbstractFactory{
    // USE THIS IF YOU NEED TO SHARE THE VECTOR AMONGST MULTIPLE FACTORY INSTANCES
    public ConcreteFactory(Vector vector){
        super(vector);
    }
    // OR USE THIS IF THE VECTOR IS SPECIFIC TO THE FACTORY
    public ConcreteFactory(){
        super(new Vector());
    }    

    public Circle createCircle(){
        Circle circle = new Circle();
        getVector().add(circle);
        return circle;
    }
}

the use of final on the instance variable is a good idea for this sort of thing - it keeps you from accidentally changing the variable elsewhere in your code. But that is optional. The key change that I made is adding vector to the constructor of the abstract base class, then passing it in from the super class.

Answer 3

static attributes are just not meant to be used that way.
statics are something, that is available only once during runtime.
In your case that means that all factories deriving from your AbstractFactory will share this single vector.

See this example:

ConcreteFactory a = new ConcreteFactory();
ConcreteFactory b = new ConcreteFactory();
a.createCircle();
b.createCircle();

Both objects a and b will now have two entries in the vector, since they share the same, static vector.

Also do I think, that

protected void setVector(Vector v){
    this.vector = v;
}

is illegal, because vector is not an attribute of the instance of the Factory, but an attribute of the Factory itself!

Adding to that it is just a bad, error prone (try debugging that on a larger scale) and plain ugly style of coding.

Just trust your teacher there - he's right ;)