1

I have two classes. 1. Node 2. MixedStateManager

Node class:

Node class maintains the state of nodes in the system. One such data is state.

public class Node
   {   
      private int state;

     public void setState(int s)
     {
      state = s;
     }

     public int getState()
     {
       return state;
     }
    }

MixedStateManager:

MixedStateManager sees if all the nodes in the system are in the same state or not. If the nodes don't have the same value for state value, then the system is said to be in MixedState. I've a variable mixedState for the same. I've made it static because, if the value is changed by one instance of the class, then it should be visible by others. Another thing to note is MixedStateManager is called by different threads which change the value of state of nodes.

public class MixedStateManager{

public static boolean mixedState;

public boolean isMixedState(){
 return mixedState;
}

public void setMixedState(){
// get all instances of Node class;
   if state of all instances are same 
     mixedState = true;
   else
     mixedState = false
}
}

Question: As MixedStateManager has only one data which is static, does it make sense to instantiate this class and then set value. Or can I make all methods static too?

Responses will be appreciated. Thanks

5
  • 1
    How many instances are being checked for mixed state, and how often do you need to know if mixed state is achieved? – Robert Harvey Jul 24 '15 at 14:29
  • On another note, I think your naming is a bit wonky. I would expect "mixed state" to indicate that the states differ, not that they are the same. – Robert Harvey Jul 24 '15 at 14:31
  • @Robert All the instances. But the system can have minimum three to maximum five instances – Praveen Hassan Jul 24 '15 at 14:31
  • If it's just five, then I would check all five every time, and avoid the static boolean altogether. – Robert Harvey Jul 24 '15 at 14:32
  • An example, fully static class: stackoverflow.com/questions/5574241/… – Deduplicator Jul 24 '15 at 17:19
5

I would argue that no class should ever have static data. That lies the road to testing hell and non-threadsafe code.

Also:

  1. Only make methods static if they have zero side-effects (ie only process the data passed as parameters in a deterministic fashion).
  2. Do not couple classes, such as having setMixedState rely on accessing Node, pass the method what it needs.

Instead have a method like:

public static bool areNodesMixedState(Node[] nodes) {
    if (state of all instances are same) {
        return true;
    else
        return false
    }
}
5
  • 1
    "Never" is a strong word. I use a static variable for logging. I also use one when I want to share data between my class instances. – Robert Harvey Jul 24 '15 at 14:37
  • @david-arno That's a good idea. But I've a question. The method isMixedMode is called at many places. So, at all those places the callee need to get all the nodes and then pass it. So, will it not be better to do it within the method. – Praveen Hassan Jul 24 '15 at 14:43
  • @David Arno, I really want to upvote you because of the "no class should ever have static data" statement I can't, That's just plain wrong. There are good cases when static data is perfectly fine. The rest of your response is exactly what I would do. – Alexus Jul 24 '15 at 17:40
  • @PraveenHassan, that rather begs the question "why is isMixedMode called in many places? This could be a warning of both too much coupling and of too little separation of concerns. – David Arno Jul 25 '15 at 10:39
  • @Alexus. There may well be edge cases, such as logging, where static state can be useful. I find it better though to start from an absolute position (eg no static state) and then require people to argue in peer review why they should be allowed an exception. I find this encourages people to think about, and ask about, alternative solutions at a much earlier stage. – David Arno Jul 26 '15 at 7:19
1

You can make it all static, but that will result in (arguably) unnecessary stronger coupling & dependency between the clients of the manager and the manager.

Thus, I agree with @DavidArno on avoiding static state as much as possible.

As @RobertHarvey noted, for logging, I use a static variable to hold a reference to a logger object because maintaining that reference everywhere you might want to add logging can be overly burdensome.

You already have the manager as an object, which is good, so if you need state for the manager, you should move it from the class to the object (by removing the static declaration).

0

A static class with only static methods and static data is nothing more or less then a collection of global variables and a library of functions disguised as something resembling OO.

Such a class need never be instantiated because the result will not be an object, just an empty shell.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.