Reduzierung der zyklomatische Komplexität einer Java-Methode

Question

ich die folgende Methode haben:

private void setClientAdditionalInfo(Map map, Client client, User user) { 

    Map additionalInfo = (Map) map.get("additionalInfo"); 

    if (checkMapProperty(additionalInfo, "gender")) { 
     client.setGender(additionalInfo.get("gender").toString()); 
    } 
    if (checkMapProperty(additionalInfo, "race")) { 
     client.setRace(additionalInfo.get("race").toString()); 
    } 
    if (checkMapProperty(additionalInfo, "ethnicity")) { 
     client.setEthnicity(additionalInfo.get("ethnicity").toString()); 
    } 
    ..... 

12 mehr, wenn Aussagen in ähnlicher Weise verwendet werden. Der einzige Unterschied ist ein anderer Setter-Methodenname und ein anderer Parameter. Nun, da das gleiche Muster immer wieder wiederholt wird, gibt es eine Möglichkeit, die Komplexität des Codes zu reduzieren?

Answer 1

Nicht leicht, und nicht ohne Verwendung von Reflexion.

Mithilfe der Reflektion können Sie die Werteliste durchlaufen und die entsprechende Methode im Clientobjekt aufrufen. Das würde die Komplexität beseitigen und sauberer/robuster sein. Allerdings würde es auch langsamer funktionieren.

Grundsätzlich haben Sie den Fall, in dem Sie fast, aber nicht ganz die gleiche Operation immer und immer wieder tun, das ist immer schwierig.

Answer 2

Ich würde eine enum verwenden, um den Setter mit dem Schlüssel zur Karte zu verbinden, mit dem name().toLowercase() als Schlüssel.

enum Setter { 

    Gender { 

       @Override 
       void set(Client client, Thing value) { 
        client.setGender(value); 
       } 

      }, 
    Race { 

       @Override 
       void set(Client client, Thing value) { 
        client.setRace(value); 
       } 

      }, 
    Ethnicity { 

       @Override 
       void set(Client client, Thing value) { 
        client.setEthnicity(value); 
       } 

      }; 

    void setIfPresent(Client client, Map<String, Thing> additionalInfo) { 
     // Use the enum name in lowercase as the key. 
     String key = this.name().toLowerCase(); 
     // Should this one be set? 
     if (additionalInfo.containsKey(key)) { 
      // Yes! Call the setter-specific set method. 
      set(client, additionalInfo.get(key)); 
     } 
    } 

    // All enums must implement one of these. 
    abstract void set(Client client, Thing value); 
} 

private void setClientAdditionalInfo(Map map, Client client, User user) { 
    Map additionalInfo = (Map) map.get("additionalInfo"); 
    for (Setter setter : Setter.values()) { 
     setter.setIfPresent(client, additionalInfo); 
    } 
} 

Answer 3

Unter der Annahme, dass die Felder von Client können null eingestellt werden, wenn die Karte nicht eine Eigenschaft enthalten, können Sie eine Klasse erstellen:

class MapWrapper { 
    private final Map map; 

    MapWrapper(Map map) { 
     this.map = map; 
    } 

    String get(String key) { 
     if (checkMapProperty(map,key)) { 
      return map.get(key).toString(); 
     } else { 
      return null; 
     } 
     // or more concisely: 
     // return checkMapProperty(map,key) ? map.get(key).toString() : null; 
    } 
} 

Und dann

private void setClientAdditionalInfo(Map map, Client client, User user) { 

    Map additionalInfo = (Map) map.get("additionalInfo"); 
    MapWrapper wrapper = new MapWrapper(additionalInfo); 

    client.setGender(wrapper.get("gender")); 
    ... 
} 

Wenn jedoch die Felder Client so belassen werden sollen, wie sie sind, wenn es keinen entsprechenden Schlüssel in der Karte gibt, hilft das nicht.

Answer 4

Sie können dies mit Java 8 funktionalen Schnittstellen tun. Es wird zumindest die wiederholten bedingten Anweisungen los.

private void doRepetitiveThing(Map info, String key, Consumer<String> setterFunction) { 
    if(checkMapProperty(info, key)) { 
     setterFunction.accept(info.get(key).toString()); 
    } 
} 

private void setClientAdditionalInfo(Map map, Client client, User user) { 

    Map additionalInfo = (Map) map.get("additionalInfo"); 

    doRepetitiveThing(additionalInfo, "gender", client::setGender); 
    doRepetitiveThing(additionalInfo, "race", client::setRace); 
    doRepetitiveThing(additionalInfo, "ethnicity", client::setEthnicity); 
    ..... 

Answer 5

Nicht sicher, ob dies tatsächlich die zyklomatische Komplexität reduziert, aber es macht den Code hübscher. Dies ist einfacher, mit Java 8.

private void setClientAdditionalInfo(Map<String, Object> map, Client client, User user) { 
    Map<String, ?> additionalInfo = (Map<String, Object>) map.get("additionalInfo"); 
    setIfPresent(additionalInfo, "gender", client::setGender); 
    setIfPresent(additionalInfo, "race", client::setRace); 
    setIfPresent(additionalInfo, "ethnicity", client::setEthnicity); 
} 

private void <T> setIfPresent(Map<String, ?> additionalInfo, 
           String property, 
           Consumer<T> consumer) { 
    if (checkMapProperty(additionalInfo, property)) { 
     consumer.apply((T)additionalInfo.get(property)); 
    } 
} 

Wenn Sie wollten, könnten Sie eine Map<String, Consumer<?>> machen die wiederholten Anrufe zu vermeiden. für jeden

private static Map<String, ClientSetter> setters = new HashMap<>(); 

interface ClientSetter { 
    void set(Client client, Object value); 
} 

static { 
    setters.put("gender", new ClientSetter() { 
     @Override 
     public void set(Client client, Object value) { 
      client.setGender(value.toString()); 
     } 
    }); 
    setters.put("race", new ClientSetter() { 
     @Override 
     public void set(Client client, Object value) { 
      client.setRace(value.toString()); 
     } 
    }); 
    setters.put("ethnicity", new ClientSetter() { 
     @Override 
     public void set(Client client, Object value) { 
      client.setEthnicity(value.toString()); 
     } 
    }); 
    // ... more setters 
} 

Iterate über die verfügbaren Eigenschaften und Aufrufen der set Methode zur Verfügung:

Answer 6

nur als eine Übung für den Zweck OPs Beantwortung der Frage, würde ich ein Map Zuordnung der Eigenschaften mit einem Einrichter Schnittstelle erstellen in der setters Karte:

private void setClientAdditionalInfo(Map map, Client client, User user) { 

    Map additionalInfo = (Map) map.get("additionalInfo"); 

    List<String> additionalInfoKeys = new ArrayList<>(additionalInfo.keySet()); 
    additionalInfoKeys.retainAll(setters.keySet()); 
    for (String key: additionalInfoKeys) { 
     Object value = additionalInfo.get(key); 
     setters.get(key).set(client, value); 
    } 
} 

PS: Natürlich ist dies nicht für die Produktion Code vorgeschlagen. Das Kopieren aller Elemente einer Sammlung in eine Liste zum Überschneiden der beiden Mengen - um Bedingungen im getesteten/geschriebenen Code zu verhindern - ist ziemlich teuer.