CODEWITHSUNDEEP
javadesign-patterns
Menelaos
Menelaos

Reputation: 26004

What is this design (or anti-) pattern and more importantly is there a better way?

I'm receiving from a webservice a list of key-value pairs, and have inherited the following code:

public String iconValue = null;
... (over 50 class variables assigned in MyObject constructor below)

public MyObject(List<Attribute> attrs) {

    String attrName, attrValue;

    for (Attribute a : attrs) {
        try
        {
            attrName = a.getName();
            attrValue = a.getValue();

            if (attrValue == null || "".equals(attrValue.trim()))
                continue;

            if (ICONS.equals(attrName)) {
                //Do something including assignment
                this.iconValue = attrValue;
            }
            else if (URL.equals(attrName)) 
            {
                //Do something including assignment
            }
            else if (...)  A giant list of over 50 different attributes hardcoded
            {
                //Do something including assignment
            }

            ...

So,except for keeping a hashmap - is there a better way than the above to keep hard coded variables within the class and use this "when-if" pattern.

Also,does this pattern have a name?

Upvotes: 2

Views: 160

Answers (4)

zw324
zw324

Reputation: 27200

One way I can think about is to use ENUMs and dynamically dispatch the works to each of the ENUM object, instead of doing a huge if else, esp. since ENUMs can be looked up by their names.

That would be like a strategy pattern.

For example:

  1. Implement an ENUM to have a method doJob() for each of the instances;
  2. Use the valueOf() method to dispatch the works.

Code sample:

public enum Strategies {
    URL {
        @Override
        public void doJob(MyObject mo) {
                // do the work
        }
    },
    ICONS {
        @Override
        public void doJob(MyObject mo) {
                // another work
        }
    };
    public abstract void doJob(MyObject mo);
}

And when using it,

try {
    Strategies.valueOf(attrName).doJob();
} catch (IllegalArgumentException e) {
    // ENUM does not exist, illegal parameter
}

Upvotes: 4

Mikkel L&#248;kke
Mikkel L&#248;kke

Reputation: 3749

I don't think it has a name, but you could call it "using polymorphism wrong" (if type safety is a concern). It depends on whether you have a well defined data contract or not. Is the data you're receiving a proper object, or just "random" data?

If it's a proper object I would create a concrete representation and use something like Dozer (or if you don't want to be tied down wit dependency, roll your own mapper using reflection) to convert between them.

If it's more or less random data, I'd just use a Map, or similar data structure.

Upvotes: 0

Stephen C
Stephen C

Reputation: 718926

Does this pattern have a name?

Nope.

In Java 7 you can express that as:

  switch (attrName) {
  case ICONS:
      //Do something including assignment
      break;
  case URL: 
      //Do something including assignment
      break;
  // and so on
  }

... provided that ICONS, URL and the other strings are compile-time constants.

That is more concise and more robust. It is also (probably) more efficient because the switch can most likely be implemented using hashing.

Upvotes: 1

Miquel
Miquel

Reputation: 15675

If you want to take a different action for each possible value of attribute, you will end up with something about that verbose, I'm afraid. Some improvements though:

If you are using Java7 or above, you can now use switch statements with Strings (link)

If you are not, you could create an Enum that has a static method that returns an Enum element you could switch on. It's no performance improvement, but it might help with readability of your code.

Upvotes: 1

Related Questions