CODEWITHSUNDEEP
java
ada
ada

Reputation: 321

Is a public setter necessary for private static variable?

/* Atleast is it needed in this case?*/
 public class Service
{       
         private static List<String> subscribedFields;
    private static List<String> unsubscribedFields;
         //---------------------------------  
         // is this necessary?
         //---------------------------------
    public static void setFields(List<String> subscribedFields, List<String> unsubscribedFields)
    {
        Service.subscribedFields = subscribedFields;
        Service.unsubscribedFields = unsubscribedFields;
    }
    public static List<String> getSubscribedFields()
    {
        return subscribedFields;
    }
    public static List<String> getUnsubscribedFields()
    {
        return unsubscribedFields;
    }
}
// some other class
public class User{
     // Is this not enough to change the lists? Isn't the setter redundant? 
     Change(Service.getSubscribedFields());
     Change(Service.getUnsubscribedFields());

}

Upvotes: 3

Views: 4919

Answers (4)

JasCav
JasCav

Reputation: 34652

No, a public setter is not always needed for a private variable. The idea of providing public setters (and getters, for that matter) is based on - what external entities such as classes need access to the insides of the particular piece of code you are writing. Getters and setters provide that public interface for that to happen. However, you don't necessarily NEED to provide a public getter or setter to every private variable you create as that private variable may only exist for internal, private use to the class.

You'll have to decide if you specifically need to give access to your private variables based on the particular needs of your code.

Update Based on ada's Question in Comments

You could give the user access to your lists directly (yes - they can use the getter and then edit the list). This may work if you trust the users of your code. However, for various reasons, you may not want to give them direct access to your list like that (especially because it gives them free reign to do what they want to do, it can create problems if you have threading within your application, etc). In that case, you should provide interfaces into the underlying list. For example, in your class, instead of providing a getSubscribedFields(), you may want to provide methods like:

// Pseudocode
public String getSubscribedField(int i) {
  return subscribedFields.Get(i);
}

public String addSubscribedField(String field) {
  subscribedFields.Add(field);
}

Hopefully that helps clarify things a bit for you.

Upvotes: 6

Tony
Tony

Reputation: 1254

Your class seems to be very prone to thread safety problems. I also question the relevance of your need to put your List as static variables.

Another thing, your setter isn't in line with JavaBeans setters & getters standards, you might have problem if you want to integrate with some common frameworks.

I suggest you a variation of your class. I refactored it in order to keep the responsibility of the class is to hold the subscriptions.

If you use a dependency injection framework like Spring or Guice, you could simply make a class like this one, and inject it to the classes that need this object.

    public class SubscriptionServiceUsingDependencyInjection {

    private final Set<String> subscribedFields = new CopyOnWriteArraySet<String>();

    public boolean isSubscribed(String field_) {
        return subscribedFields.contains(field_);
    }
    public void subscribe(String field_) {
        subscribedFields.add(field_);
    }
}

Otherwise If you really need a Singleton, you may use an enum to achieve the same goal :

    public enum SubscriptionServiceUsingASingleton {
    INSTANCE;
    private final Set<String> subscribedFields = new CopyOnWriteArraySet<String>();

    public boolean isSubscribed(String field) {
        return subscribedFields.contains(field);
    }
    public void subscribe(String field) {
        subscribedFields.add(field);
    }
}

The CopyOnWriteArraySet will prevent concurrency problem if you are running this in a multithreaded environment.

Upvotes: 1

Stan Kurilin
Stan Kurilin

Reputation: 15792

No you shouldn't. And even more you should avoid static state.

Upvotes: 1

Neil
Neil

Reputation: 5780

The more popular choice in this case is to use a Singleton which you initialize once with the fields. However, not even Singleton is really a good idea, but to do otherwise requires that we know a bit more about what you're trying to do. In most cases you can get around using static instances by making it a member of a class with a long lifetime. For example, if these fields were related to database fields, you'd associate it with a table class which holds information pertaining to a database table for instance.

Really it depends on what you're trying to accomplish.

Upvotes: 1

Related Questions