views:

144

answers:

5

Here's an example of a static method that is used in a web application. As you can see, the String[] allergensArr gets insantiated each time that this method is called. It's threadsafe since it's in a static method but it's an expensive call.

What are some other ways that the allergensArr[] can be used so that it's not instantiated each time the method is called.

I was considering the following options.

  • Have a static constructor that initialzies a static final String[]
  • Use a singleton (though this would block a lot of people)

This will be a constant array that will not change in the lifetime of an instantiated server.

public class UserHealthConcernsManager {
    public static String[] getAllergensFlag () {

     String[] allergensArr = new String[12];

     allergensArr[0] = "x";
     allergensArr[1] = "y";
     allergensArr[2] = "w";
                 _SNIP_
                return allergensArr;
     }
}
A: 

Sometimes the problem scope defined can be too narrow to come up with a good optimization.

If your problem is that you NEED to move those items from the passed in hash map to an array, it's not going to get too much more efficient (although you probably want to start with an array like this: new String[] {"ADDED_SUGARS_FREE_FLAG", "EGG_FREE_FLAG", ...} and iterate over it so you don't have all those duplicated lines.

So to get a better optimization you might have to zoom out a level or two. Why are you storing them in an array, couldn't you just copy the user hashmap in part or in whole? Can the user hashmap be made immutable so you could just make a pointer copy of it and not even bother with extracting value?

Or better yet, can you wrap HashMapSupport with a more intelligent collection that solves the problems of all classes using it.

I can't really answer any of these without knowing a lot more about your code, but that's the kind of stuff I'd be looking at.

After your edit: You already changed the problem a bit. What you now have is equivalent to:

return new String[]{"w", "x", "y", ...}

Are you sure you didn't simplify out part of the problem?

Bill K
+2  A: 
  1. Static doesn't mean threadsafe. If "user" (HashMapSupport) is shared across multiple threads...
  2. It doesn't look terribly expensive.
  3. You could store this sort of information in the Session object, if you really feel you need to.
  4. If you need to guarantee that the string array can't be modified, then you need to wrap the string array in some other object to guarantee that the stored object is immutable (e.g., only getter methods).

Edit: Yargh. As noted by others, you've changed the problem considerably. If you want to guarantee the array is immutable, then see point #4.

hythlodayr
A: 

Just initialize the array on web-app startup and put in the servlet context. See ServletContextListener and ServletContext.

+1  A: 

A static constructor seems to be the obvious solution.

static String[] allergensArr = {"x", "y", "w", ...9 more...}

You do need to make sure no one reassigns the static field or modifies the array during the lifetime of your application.

Update: if you really care about making sure no clients tamper with it, you could do:

final static List<String> x = Collections.unmodifiableList(Arrays.asList("x", "y", "z", ...));
Keith Randall
the first problem can be alleviated by using static final String[] ...
james
Note that a static method which returns a reference to an array such as in this manner is opening it up for other classes/threads to modify the contents of this array.
matt b
@james, Keith meant "make sure no clients change the values in the array" adding the final keyword won't prevent clients from executing code like `getAllergensFlag()[0] = "haha";`
rsp
+3  A: 

Have you considered using Lists instead of arrays of references?

The code could be reduced to:

public static final List<String> allergensFlag =
    Collections.unmodifiableList(Arrays.asList(
        "x",
        "y",
        "w",
        ...
    ));

If you really, really wanted old fashioned arrays, then the cost of cloning is tiny.

private static final String[] allergensFlag = {
    "x",
    "y",
    "w",
    ...
};

public static String[] getAllergensFlag () {
    return allergensFlag.clone();
}
Tom Hawtin - tackline
This sounds like the approach I want.
Shaun F