2

I've the following Interface.

public interface ValueInterface
{
  long  getValue();
}

And the class:

public class ValueCreator implements ValueInterface
{
  private static long value  = 0;

  public void setValue(long valueInDollar)
  {
    value= valueInDollar;
  }

  @Override
  public long getValue()
  {
    return value;
  }
}

The static code analyzer findbugs warned me not to set a static field in a non-static method. If I make the set method static, I can't make the getter static as it is an interface overridden method.

So, is it okay to have a static setter and non-static getter or what could be a better approach?

  • 1
    Are you sure that the field has to be static in the first place? Non-static setter and getter give the impression that the value you are retrieving or changing belongs to the instance. – Karsten Hahn May 13 '14 at 7:24
  • @Veitch Yes, the field has to be static, as it is same for all instances. I did change the setter to static method but can't do the same for getter as it's a overridden method of interface. – Praveen Hassan May 13 '14 at 7:30
  • I personally would prefer to be consistent and not mix up static and non-static getter and setter. However, there might be a better approach if the context was known. – Karsten Hahn May 13 '14 at 7:39
3

No, it is not a good idea to have a non-static getter for a static field.

The problem is that you can have multiple instances of ValieCreator, but they will all share the same static value field, so all instances will always return the same value and if you change the value through one instance, the modification is visible in all.
This is compounded by the fact that the getter is declared in the ValueInterface interface. Suppose you have this code:

void SillyMethod(ValueInterface i, ValueCreator c)
{
    if (i != c)
    {  // different objects
        long val = i.getValue();
        c.setValue(val+1);
        if (val != i.getValue())
        {
            // Huh? Setter on c changed the value on a different object?
        }
    }
}

It is better to make the value field non-static. If you must ensure there can only ever be one ValueCreator, then you can use the Singleton pattern for that (but first ensure that there is no scenario imaginable where there might be two or more ValueCreator objects).

0

FindBugs does not actually find bugs (or at least not only bugs). It finds things that are strange. So if what you really want is have that value static and need to implement interface that gets value and it should return that value, you cannot do differently than non static method accessing static field. If setter is not required by that interface, you can use static or non-static setter, it's up to you. But non-static method working with static data is something strange (not wrong), so i'd prefare static setter.

Are you sure that you want/need that value to be static? (but that's for different discussion)

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.