2
\$\begingroup\$

Code below is scattered all over the code base :

if (StringUtils.contains(manager.getName, "custom")){

}

It just checks if an object attribute contains a predefined String and if it does, enter condition.

Is there a more elegant way of achieving above, perhaps using an enum?

\$\endgroup\$
  • 1
    \$\begingroup\$ Is the string being searched for always the same? \$\endgroup\$ – Konrad Rudolph Jul 19 '12 at 12:43
  • \$\begingroup\$ @Konrad Rudolph yes its always the same \$\endgroup\$ – blue-sky Jul 19 '12 at 13:37
3
\$\begingroup\$
  1. It seems data envy. I'd create nameContains method in the Manager class.

    public boolean nameContains(final String searchText) {
        if (StringUtils.contains(name, searchText)) {
            return true;
        }
        return false;
    }
    
  2. Instance variables (fields) should be private. See: why instance variables in java are always private

| improve this answer | |
\$\endgroup\$
  • 6
    \$\begingroup\$ if (cond) return true; else return false; => return cond; \$\endgroup\$ – Konrad Rudolph Jul 19 '12 at 12:43
  • \$\begingroup\$ +1 for the link to data envy. However, as Konrad suggested, the body of this method should be a one-liner. \$\endgroup\$ – Leonid Jul 22 '12 at 18:27
2
\$\begingroup\$

You can use a static reference for "custom" :

    // In References.class declared 'final'
public final class References{
    static String refSearched = "custom";

    public static boolean methodControl1(final String s){
        return s.contains(refSearched);
    }    
    .... // other fields/methods
}

    // In other classes
    if(References.methodControl1(newStringToSearchIn)){
        ..;
    }
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Are the strings like "custom" pre-defined? If the number of such strings are limited, then you would also profit by making them into methods such as Manager.hasCustom() if not, you can go for Manager.has(Manager.CUSTOM) where CUSTOM could be an attribute in the Manager class.

Consider memoizing the lookup.

class Manager {
  public final String CUSTOM = "custom";
  public boolean has(String key) {
    if (!map.containsKey(key)) map.put(StringUtils.contains(name, key))
    return map.get(key);
  }
  public boolean hasCustom() { return has(CUSTOM); }
}

Finally, if your class takes decisions based on a flag in another class, you might want to rewrite it to reduce coupling. And perhaps use a pattern like Strategy.

| improve this answer | |
\$\endgroup\$

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.