11

I havent used a lot of static methods before, but just recently I tend to use more of them. For example if I want to set a boolean flag in a class, or acess one without the need to pass the actual object through classes.

For example:

public class MainLoop
{
    private static volatile boolean finished = false;

    public void run()
    {
        while ( !finished )
        {
            // Do stuff
        }

    }
    // Can be used to shut the application down from other classes, without having the actual object
    public static void endApplication()
    {
        MainLoop.finished = true;
    }

}

Is this something I should avoid? Is it better to pass a object so you can use the objects methods? Does the boolean finished counts as a global now, or is it just as safe?

TacticalCoder
  • 6,275
  • 3
  • 31
  • 39
Lucas Arrefelt
  • 3,879
  • 5
  • 41
  • 71
  • The `finished` variable is indeed "global" to the class. Java does not have truly "global" (to everything) variables. – Hot Licks Mar 02 '12 at 22:41
  • 1
    @Hasslarn: your code is broken (and I'm a bit surprised that after four answers no-one mentioned it): there's absolutely zero guarantee that when another thread shall call *endApplication()* the value shall be read as *true* by the thread running the *run()* method. You **must** use some synchronization mechanism here, like for example by declaring *finished* as *volatile*. – TacticalCoder Mar 02 '12 at 23:00
  • @Hasslarn: I edited your question and fixed your code (not that using static this way is clean or anything) by adding *volatile* to your boolean flag. – TacticalCoder Mar 02 '12 at 23:08
  • Applauds to you for noticing, kinda made the example on the fly to make my point :) – Lucas Arrefelt Mar 02 '12 at 23:30

4 Answers4

8

A problem with using a static variable in this case is that if you create two (or more) instances of MainLoop, writing code that looks like it is shutting down only one of the instances, will actually shut down both of them:

MainLoop mainLoop1 = new MainLoop();
MainLoop mainLoop2 = new MainLoop();

new Thread(mainLoop1).start();
new Thread(mainLoop2).start();

mainLoop1.finished = true; // static variable also shuts down mainLoop2 

This is just one reason (amongst many) for choosing to not use static variables. Even if your program today only creates one MainLoop, it is possible that in the future you may have reason to create many of them: for unit testing, or to implement a cool new feature.

You may think "if that ever happens, I'll just refactor the program to use member variables instead of static variables." But it's generally more efficient to pay the cost up front, and bake modular design into the program from the start.

There's no question that statics often make a quick and dirty program easier to write. But for important / complex code that you intend to test, maintain, grow, share, and use for years to come, static variables are generally recommended against.

As other answers to this question have noted, a static variable is a kind of global variable. And there's lots of information about why (generally) global variables are bad.

Community
  • 1
  • 1
Mike Clark
  • 10,027
  • 3
  • 40
  • 54
7

Yes, passing objects around is better. Using a singleton or static methods makes OO programming look like procedural programming. A singleton is somewhat better because you can at least make it implement interfaces or extend an abstract class, but it's usually a design smell.

And mixing instance methods with static variables like you're doing is even more confusing: you could have several objects looping, but you stop all of them at once because they all stop when a static variable changes.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
5

Is this something i should avoid?

In general, yes. Statics represent global state. Global state is hard to reason about, hard to test in isolation, and generally has higher thread-safety requirements.

If I want to test what happens to an object in a certain state, I can just create the object, put it into that state, perform my tests, and let it get garbage collected.

If I want to test what happens to global state, I need to make sure I reset it all at the end of my test (or possibly at the start of every test). The tests will now interfere with each other if I'm not careful about doing that.

Of course, if the static method doesn't need to affect any state - i.e. if it's pure - then it becomes somewhat better. At that point all you're losing is the ability to replace that method implementation, e.g. when testing something that calls it.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
2

In general, by making finished static like that you create a situation where there can only be one instance of your MainLoop class executing run at any one time. If there is more than one instance then setting finished will end them all -- not what is usually desired.

However, in this particular scenario, where you want to "end application", presumably meaning you want to end all instances of MainLoop, the approach may be justified.

However, the number of situations where this approach may be merited are few, and a "cleaner" way to handle this scenario would be to keep a static list of instances and work through the list, setting the instance variable finished in each instance. This allows you to also end individual instances, gives you a natural count of existing instances, etc.

Hot Licks
  • 47,103
  • 17
  • 93
  • 151
  • Yes, me myself in this particular scenario believe its justified, but it could make an expansion of the very same foundation hard as you and all the others imply. Thanks for the workaround example – Lucas Arrefelt Mar 02 '12 at 22:52