views:

682

answers:

9

I ran across a class that was set up like this:

public class MyClass {

  private static boolean started = false;

  private MyClass(){
  }

  public static void doSomething(){
    if(started){
      return;
    }
    started = true;
    //code below that is only supposed to run
    //run if not started
  }
}

My understanding with static methods is that you should not use class variables in them unless they are constant, and do not change. Instead you should use parameters. My question is why is this not breaking when called multiple times by doing MyClass.doSomething(). It seems to me like it should not work but does. It will only go pass the if statement once.

So could anyone explain to me why this does not break?

+9  A: 

The method doSomething() and the variable started are both static, so there is only one copy of the variable and it is accessible from doSomething(). The first time doSomething() is called, started is false, so it sets started to true and then does... well, something. The second and subsequent times it's called, started is true, so it returns without doing anything.

Kieron
So are saying you are guaranteed to have only one copy of this class that will be initialized and used throughout the time the application is running? My concern was that I was not sure if the same instance would be used every time, and that the variable value would not be consistent.
jschoen
+1 for right answer. Also, here's a place to get more info regarding static variables in general. Probably a good thing for you to read, bigbrother82: http://en.wikipedia.org/wiki/Static_variable
Welbog
@bigbrother82: There are multiple instances of the class, but only one copy of the started variable shared between them.
Welbog
You should precise that the variable is initialized only once for all instances of the class. So you could call doSomething() only 1 time and have an execution regardless of the number of instances of the class.
boutta
static means it behaves like there is only one copy. Therefore static methods and static variables are only in existence once when called. There is the potential issue of two calls to doSomething() at once, and both of them passing through the if statement
ck
Just make sure you don't have multiple threads calling doSomething when it may potentially not be initialized. See extraneon's answer for how to do this safely.
deterb
+1  A: 

Within static method, you are allowed to invoke or access static members within the same class.

Disregarding multiple threads scenarios, The first call to doSomething will make the boolean static variable to true, therefore, the second call will execute the code of if block which just simply exit the method.

codemeit
A: 

The code above works completely well (unless it runs in a multithreaded environment). Why do you think it should break?

bruno conde
Or it throws an exception. Then there is the question about what the "code below" actually does. Presumably more evil mutable statics.
Tom Hawtin - tackline
A: 

My understanding with static methods is that you should not use class variables in them unless they are constant, and do not change

I guess only static members can be accessed. It need not be constant!

My question is why is this not breaking when called multiple times by doing MyClass.doSomething(). It seems to me like it should not work but does. It will only go pass the if statement once

Per the existing logic. Only the first call runs the //code to be run part

Bharani
+6  A: 

There's no reason why using a static variable wouldn't work. I'm not saying it's particularly good practice, but it will work.

What should happen is:

  1. The first call is made. The class is initialised, started is false.
  2. doSomething is called. The if fails and the code bypasses it. started is set to true and the other code runs.
  3. doSomething is called again. The if passes and execution stops.

The one thing to note is that there is no synchronization here, so if doSomething() was called on separate threads incredibly close together, each thread could read started as false, bypass the if statement and do the work i.e. there is a race condition.

GaryF
+2  A: 

It's not particularly nice code - generally designs should use object instances where state changes, but there's nothing illegal with it.

My understanding with static methods is that you should not use class variables in them unless they are constant, and do not change.

You seem to have extrapolated from a design guideline to a language feature. Read one of the many Java tutorials available on line as to what is actually allowed in the language. You can use non-final static fields freely in static methods, but it leads to procedural rather than object-oriented code.

Instead you should use parameters.

It's hard to see how an started parameter would be used - if the caller knew that the process has been started, why would they call the method?

Pete Kirkham
+1  A: 

You static method is talking to a static class variable, so it should be fine. You could think of this as global code and a global variable, tho it IS in the namespace of the class.

If you tried to access a non-static member variable:

private int foo = 0;

from within the static method, the compiler will and should complain.

started is false - initial state.
MyClass.doSomething() - statered is now true
MyClass.doSomething() - started is STILL true

MyClass foo = new MyClass();
foo.started -> it's STILL true, because it's static
foo.doSomething() - not sure you can do this in Java, but if you can, it's be STILL TRUE!

Now, there are issues in the above code with thread safety, but aside from that, it appears to be working as designed.

Nic Wise
+4  A: 

The code given is not thread safe. The easy way to make this code thread safe would be to do something like

public class MyClass {

  private static AtomicBoolean started = new AtomicBoolean(false);

  private MyClass(){
  }

  public static void doSomething(){
    boolean oldValue = started.getAndSet(true);
    if (oldValue)
      return;
    }

    //code below that is only supposed to run
    //run if not started
  }
}

This should be thread safe as the AtomicBoolean getAndSet is synchronized.

Admittedly this is not an issue if you do not use threads (please note that a webapp can use quite a lot of threads handling various requests without you being aware of that).

extraneon
+1  A: 

Just remember the thumb rule that "Static variables are class-level variables and all non-static variables are instance variables". Then you won't have any confusion at all!

i.e. For static variable, All references made in code to the variable point to same memory location. And for non-static variable, new memory allocation is done whenever new instance of that class is created (so every reference made in code to the variable points to a different memory location allocated for calling class instance).

Chandan .