CODEWITHSUNDEEP
java
Genadinik
Genadinik

Reputation: 18649

How to create a hardcoded lookup that would be reused by different parts of the program?

I have what seems to me a basic issue that confuses me.

Right now I have the following bad design (it seems to me it is bad design). I have a util class which looks something like this:

public class Countries 
{
    public boolean isCountryPresent ( String c )
    {
        //public static final http://en.wikipedia.org/wiki/ISO_3166-1 
        Set<String> myStrings = new HashSet<String>();
        myStrings.add("us"); // Afghanistan
        myStrings.add("af"); // Afghanistan
        myStrings.add("dz"); // Algeria
        myStrings.add("ao"); // Angola
        myStrings.add("az"); // Azerbiajan
        ...


    if ( myStrings.contains(c))
        return true;
    else
        return false;
}

And then I check for the existance of the item like this:

Countries co = new Countries ( );
boolean isPresent = co.isCountryPresent( countryISOCode );

But I am thinking that instantiating the object each time wastes resources, no? Is there a more efficient way for doing this given that the country data does not need to be compiled more than once and does not change?

Upvotes: 1

Views: 191

Answers (3)

Jack
Jack

Reputation: 133679

You can use an enum or declare a single static instance of Countries which is initialized on startup, eg:

class Countries {
  public final static Countries instance = new Countries();

  private Set<String> myStrings;
  private Countries() {
    myStrings = new HashSet<String>();
    myStrings.add("us");
    ..
  }
}

Upvotes: 1

Kevin DiTraglia
Kevin DiTraglia

Reputation: 26078

I usually do something like this:

public class Countries 
{
     private static Set<String> myStrings = null
     public static boolean isCountryPresent ( String c )
     {
        if (myStrings == null) {
            myStrings = initializeSet();
        }

        if ( myStrings.contains(c))
            return true;
        else
            return false;
    }
    private static Set<String> initializeSet() 
    {
        //public static final http://en.wikipedia.org/wiki/ISO_3166-1 
        Set<String> countrySet = new HashSet<String>();
        myStrings.add("us"); // Afghanistan
        myStrings.add("af"); // Afghanistan
        myStrings.add("dz"); // Algeria
        myStrings.add("ao"); // Angola
        myStrings.add("az"); // Azerbiajan
        ...
        return countrySet;
    }
}

This way it initializes the set the first time the method is called, but for all subsequent calls, the old initialization is cached.

You could also declare it in the constructor, but I tend to lean toward the lazy loading approach, so the application isn't waiting for all this stuff to load right when it launches, but is loaded sort of on demand when you actually need it.

Also as mentioned in the comments this should probably all be static, unless you are always reusing the same Countries object throughout the code, otherwise it will be redone for every new Countries object you instantiate.

Upvotes: 2

Jeff
Jeff

Reputation: 12795

The simplest transition is to move the set to a class variable, and instantiate it in the constructor:

public class Countries 
{
    private Set<String myStrings = new HashSet<String();

    public Countries() {
        //public static final http://en.wikipedia.org/wiki/ISO_3166-1 
        Set<String> myStrings = new HashSet<String>();
        myStrings.add("us"); // Afghanistan
        myStrings.add("af"); // Afghanistan
        myStrings.add("dz"); // Algeria
        myStrings.add("ao"); // Angola
        myStrings.add("az"); // Azerbiajan
        ...
    }

    public boolean isCountryPresent ( String c )
    {
        return myStrings.contains(c);
    }
}

That way it's just called once, when you first construct the Countries object.

Also note that you don't need to have an if statement: contains returns a boolean, so you can just return that straight.

Edit: just noticed your second code segment. Instead of creating a new Countries object each time, just make it once, and persist it.

Alternatively, you could make this static (private static Set..., static { instead of public Countries() { and public static boolean...)

Then you just call it like Countries.isCountryPresent(SomeCountry); without instantiating a new object.

Upvotes: 1

Related Questions