Don't let features cloud your object-oriented design judgement: A static variable trap

Insights from a Java n00bie #3

By Colin Nicholls
September 2000

(This may seem contrived but there is a real-world problem I was trying to solve here^[1])

Imagine that you have a bunch of actors auditioning for a part in a film. You don't know how many actors there are, but you know that only one will be auditioning for the part at any one time. We could model this in a multi-threaded Java application with the following generic actor class:

abstract class Actor {
    static boolean performing = false;
    setPerforming( boolean b ) { performing = b; }

    public void perform() {
       if (!performing) {
           setPerforming(true);
           System.out.println("What is my motivation for this scene?");
       } else {
           System.out.println("I can't perform. Another actor is currently auditioning.");
       }
    }

    public void exitStageLeft() {
        if (performing) {
            setPerforming(false);
            System.out.println("Actor has left the stage.");
        } else {
            System.out.println("I can't leave the stage if I'm not on it!");
        }
    };
}

Assuming you are familiar with class fields and the static keyword, you'll recognize that no matter how many threads with instances of Actor you have, only one of them can be performing at any one time. (NB: We're not using synchronized here because the scenes are long and for various reasons we don't want to lock the threads.)

OK, that's fine for a generic actor. But perhaps you are holding concurrent auditions for different parts? Each actor is still only interested in one particular part, though. Good object-oriented design suggests that we keep common functionality in a generic Actor class, and subclass it for each role: 

abstract class Actor {
    static boolean performing = false;
    abstract void perform();
    abstract void exitStageLeft();
    void setPerforming( boolean b ) { performing = b; }
}

class Superhero extends Actor {
    void perform() {
        if (!performing) {
            setPerforming(true);
            System.out.println("Superhero steps into the phone booth...");
        } else {
            System.out.println("Superhero: Phone booth is occupied!");
        }
    }
    
    void exitStageLeft() {
        if (performing) {
            setPerforming(false);
            System.out.println("Superhero leaves the scene.");
        } else {
            System.out.println("Superhero hasn't arrived yet?!");
        }
    }
}

class Dancer extends Actor {
    void perform() {
        if (!performing) {
            setPerforming(true);
            System.out.println("Dancer makes her entrance...");
        } else {
            System.out.println("Dancer: Someone else is already dancing!");
        }
    }
    
    void exitStageLeft() {
        if (performing) {
            setPerforming(false);
            System.out.println("Dancer leaves the dance floor.");
        } else {
            System.out.println("Dancer has't started yet!");
        }
    }
}

Now, a simple test program to see if we can have no more than two actors perform for two different scenes at the same time:

    Actor s1 = new Superhero();
    Actor s2 = new Superhero();
    Actor d1 = new Dancer();
    Actor d2 = new Dancer();

    s1.perform();
    d1.perform();
    s2.perform();
    d2.perform();

    s1.exitStageLeft(); 
    :
    (etc)

Here's what we get printed on the console:

Superhero steps into the phone booth...
Dancer: Someone else is already dancing!
Superhero: Phone booth is occupied!
Dancer: Someone else is already dancing!
Superhero leaves the scene.

Oh dear. What happened? Why wasn't our first instance of Dancer able to audition? Basically, it's because all four objects share a common one class field, the performing flag, even though there are two instances of two different classes involved here.

Class Fields of a superclass are shared by all subclass instances

Because the static performing flag is defined at Actor superclass level, there is only one memory location for all subclasses of Actor. This makes sense when you remember that, if it were public, you could access it at any point in your code by:

if (Actor.performing) {
:

Does this mean that we can't have a common Actor superclass for our Actor class? It's almost a given that as you make your Actors more capable, there will be some common functions that are appropriate to be implemented in an Actor superclass, but maybe checking audition/queue status isn't one of them.

It's an object-oriented modelling problem

A good question to ask at this point is: Where should the business logic that indicates whether or not an audition is busy be located? It's really not the job of an Actor, but perhaps of an Audition class? Perhaps each Actor refers to which audition they are trying out for. You're probably figuring out how to alter the Java model to be a better fit already. Essentially:

class AuditionManager {
    static isFree( String audition ) { .. }
    static synchronized start( String audition ) { .. }
    static stop( String audition )   { .. }
    : 
}

abstract class Actor {
    boolean performing; 
    abstract void perform();
    abstract void exitStageLeft();
}

class Superhero extends Actor {
    void perform() {
        if (AuditionManager.isFree("superhero") {
            AuditionManager.start("superhero");
            performing = true;
            System.out.println("Superhero steps into the phone booth...");
        } else {
            System.out.println("Superhero: Phone booth is occupied!");
        }
    }
    
    void exitStageLeft() {
        if (performing) {
            AuditionManager.stop("superhero");
            performing = false;
            System.out.println("Superhero leaves the scene.");
        } else {
            System.out.println("I haven't started auditioning yet!");
        }
    }
}
:
(etc)

I leave the internal structure of AuditionManager as an exercise for the student. Essentially, we're still using static class fields to store and manipulate a single status flag, but we've moved it out into its own class, where it should have been from the start.

---

(1) I had a multithreaded scheduler spawning task threads of various types, with a lot of common code shared by the Task class definitions.

previous insight