2016-08-08 90 views
3

Ich schreibe etwas Code und ich habe dieses Gefühl von Urgh, die ich bekomme, wenn es hässlich und unelegant fühlt, aber ich kann nicht sehen, einen sofortigen Weg, um es zu vermeiden.Vermeidung der ständigen Überprüfung, um sicherzustellen, Json-Objekt enthält Elemente

Ich habe ein JSON-Objekt, das ich von einer dritten Partei bekomme. Ich weiß, was zu erwarten ist, aber ich kann nicht sicher sein, dass jedes Element auf jeden Fall da sein wird, so muss ich prüfen, ob es so gibt:

if (object.has(ELEMENT)) { 
    JsonObject element = object.get(ELEMENT); 
} 

Das Problem ist, dass ich manchmal ziemlich tief in die gehen Objekt und es beginnt sich hässlich zu fühlen, wenn ich so viele verschachtelte ifs bekomme. Hier ein Beispiel:

private boolean lineExists(JsonArray orders, Line lineItem) { 
    final String LINE_ITEMS = "lineItems"; 
    final String ELEMENTS = "elements"; 
    final String NOTE = "note"; 
    boolean exists = false; 

    log.debug("Checking if line item already exists in order..."); 

    // if there is no order payload then the order hasn't already been posted so there can't be a duplicate line item 
    if (orders != null) { 
     for (int i = 0; i < orders.size(); i++) { 
      JsonObject order = orders.get(i).getAsJsonObject(); 
      if (order.has(LINE_ITEMS)) { 
       JsonObject lineItems = order.get(LINE_ITEMS).getAsJsonObject(); 
       if (lineItems.has(ELEMENTS)) { 
        JsonArray elements = lineItems.get(ELEMENTS).getAsJsonArray(); 
        for (int j = 0; j < elements.size(); j++) { 
         JsonObject existingLine = elements.get(j).getAsJsonObject(); 
         if (existingLine.has(NOTE)) { 
          String note = existingLine.get(NOTE).getAsString(); 
          // the note may change after this comparison so just check if the ID is contained in the 
          // note 
          if (note.contains(lineItem.getNote())) { 
           exists = true; 

           log.warn("Line item with note containing '{}' already exists in order.", 
             lineItem.getNote()); 
          } 
         } 
        } 
       } 
      } 
     } 
    } 

    return exists; 
} 

Ich weiß, dass ich einige der Tests in eigene Methode wie folgt aufteilen:

private boolean lineExistCheck(JsonObject order) { 
    final String LINE_ITEMS = "lineItems"; 
    final String ELEMENTS = "elements"; 

    return order.has(LINE_ITEMS) && order.get(LINE_ITEMS).getAsJsonObject().has(ELEMENTS); 
} 

Ich frage mich nur, wenn es ein Design-Muster oder eine Art und Weise des Denkens, das mir helfen würde, in diesem Fall besseren Code zu schreiben.

+0

Danke für die späten aber immer herzlich willkommen zu akzeptieren! – GhostCat

+0

@GhostCat Total dachte ich hätte das schon gemacht! Entschuldige die Verzögerung (oops). – SBmore

+0

Kein Grund zum Mitleid. Wenn Sie länger herum sind, sind solche unerwarteten Dinge wirklich etwas, das Sie genießen; es fühlt sich irgendwie wie "free lunch" an (du siehst, die "Anstrengung" hat vor einigen Wochen stattgefunden und jetzt gibt es einen überraschenden Gewinn daraus ;-) – GhostCat

Antwort

3

Sie möchten über die single layer of abstraction principle lesen.

Im Wesentlichen würde die Idee sein, dass jede Sache, die erfordert, dass Sie nach rechts einrücken ... sollte besser in seine eigene Methode gehen. Sie setzen Ihre Tests nicht nur in separate Methoden ein; Du gehst noch weiter.

Und Randnotiz: Endlich so verwenden ... kauft dir nichts. In meinen Augen (aber das ist eine Meinung über Stil!) Ist dies nur Verschwendung, die Ihre CPU ohne guten Grund zum Brennen bringt. Einen Schritt weiter gehen; es könnte mehr Sinn machen, diese lokalen Konstanten in globale statische umzuwandeln.

Die andere Sache (die natürlich knifflig ist, angesichts der Tatsache, dass Sie an einem JsonObject arbeiten) ... dieser Code ist ein "nettes" Gegenbeispiel zu Tell Dont Ask. Der Punkt ist: Ihr Code fragt den internen Status anderer Objekte ab; um darauf basierende Entscheidungen zu treffen. Im Wesentlichen ist das prozedurale Programmierung. In echten OO, fragen Sie nicht ein Objekt für einige Informationen, um dann etwas zu tun; Nee; Sie einfach erzählen dieses Objekt zu tun, was auch immer benötigt wird. Aber wie gesagt; das hilft hier nicht wirklich; angesichts der Tatsache, dass es sich um ein generisches JsonObject handelt; und nicht ein "echtes" Objekt, das eine bekannte und gut definierte Schnittstelle hat, die Sie verwenden könnten, um es "zu sagen", was zu tun ist.

+0

Danke für die Dokumentation und den Code Review. – JeanMel

+0

Danke @ GhosttCat für die Vorschläge. Ich untersuche jetzt das SLA-Prinzip und hoffe, es in meinen Projekten umzusetzen. Ich wusste auch nicht, dass ich das finale Schlüsselwort falsch verwendete, also werde ich etwas zur richtigen Zeit lesen, um es auch zu benutzen. – SBmore

+0

Sie sind herzlich willkommen! – GhostCat

1

Was ist mit dem Erstellen eines Wrappers über JsonObject, die Methoden für getLineItems/getElements hätten, die alle ein leeres Array zurückgeben würden, falls es keine Werbebuchungen oder Elemente gibt. In diesem Fall können Sie Ihren Code schreiben, ohne zu prüfen, ob bestimmte Eigenschaften vorhanden sind.

In C# würde man eine Bibliothek verwenden, die die Konvertierung zwischen JsonObject und Ihrer Domänenklasse see this small example implementiert, um eine Idee zu bekommen. Ich bin ziemlich sicher, dass eine ähnliche Funktionalität in einer Bibliothek für Java existiert.

1

Java 8 Streaming-API kann beim Glätten dieser verschachtelten Schleifen helfen.Ohne zu wissen, welche JSON Bibliothek haben Sie verwenden und nicht zu wollen, um herauszufinden, ich Ihre Struktur in verschachtelte Listen übersetzt und Karten:

JsonArray { 
    JsonObject { 
    String => JsonObject { // LINE_ITEMS 
     String => JsonArray { // ELEMENTS 
     JsonArray { 
      JsonObject { 
      String => String // NOTE 
      } 
     } 
     } 
    } 
    } 

entspricht

List< 
    Map<String,    // LINE_ITEMS 
    Map<String,    // ELEMENTS 
     List< 
     Map< 
      String, String  // NOTE 
     > 
     > 
    > 
    > 
> 

ich nicht den Code Refactoring hat weiter (wie macht die Saiten Konstanten in der endgültigen statische Variablen, die sehr zu empfehlen) sind nur in dieser Frage konzentriert: Entfernen der verschachtelten Schleifen:

private static boolean lineExists(Optional<List<Map<String, Map<String, List<Map<String, String>>>>>> ordersOrNull, Line lineItem) { 
    final String LINE_ITEMS = "lineItems"; 
    final String ELEMENTS = "elements"; 
    final String NOTE = "note"; 
    MutableBoolean exists = new MutableBoolean(false); 

    // if there is no order payload then the order hasn't already been posted so there can't be a duplicate line item 
    ordersOrNull.ifPresent(orders -> { 
    orders.stream().filter(order -> order.containsKey(LINE_ITEMS)) 
     .map(order -> order.get(LINE_ITEMS)) 
     .filter(lineItems -> lineItems.containsKey(ELEMENTS)) 
     .flatMap(lineItems -> lineItems.get(ELEMENTS).stream()) 
     .filter(element -> element.containsKey(NOTE) && element.get(NOTE).contains(lineItem.getNote())) 
     .map(existingLine -> existingLine.get(NOTE)) 
     .findAny() 
     .ifPresent(n -> { 
     System.out.println(String.format("Line item with note containing %s already exists in order.", lineItem.getNote())); 
     exists.set(); 
    }); 
    }); 
    return exists.get(); 
} 

Ich habe einen Optional verwendet, um den Null-Check zu entfernen und klarzustellen, dass eine Null passieren kann. Ich habe einen MutableBoolean verwendet, um den Ergebniswert zu setzen, da lokale Variablen, die in Lambdas verwendet werden, effektiv final sein müssen.

Wenn Sie fragen, gibt es eine MutableBoolean in Apache Commons-lang, aber es ist nicht wirklich wichtig im Zusammenhang mit der Frage - es ist nur ein Detail der Implementierung. Der einzige Zweck dieses Snippets besteht darin, zu demonstrieren, wie die Streaming-API die Verwendung von geschachtelten Schleifen und bedingten Anweisungen vollständig überflüssig macht.

Aus Gründen der Vollständigkeit hier ist meine Beispielimplementierung:

public static class MutableBoolean { 
    boolean value; 
    public MutableBoolean(boolean initialValue) { 
    this.value = initialValue; 
    } 
    public void set() { 
    value = true; 
    } 
    public void reset() { 
    value = false; 
    } 
    public boolean get() { 
    return value; 
    } 
} 

Und ein sehr einfaches Programm den Code oben zu testen:

interface Line { 
    String getNote(); 
} 

public static void main(String[] args) { 
    Line line1 = new Line() { 
    public String getNote() { 
     return "nothing"; 
    } 
    }; 
    Line line2 = new Line() { 
    public String getNote() { 
     return "something"; 
    } 
    }; 

    Map<String, String> existingLine = new HashMap<>(); 
    existingLine.put("note", "something"); 
    List<Map<String, String>> elements = new ArrayList<>(); 
    elements.add(existingLine); 
    Map<String, List<Map<String, String>>> lineItems = new HashMap<>(); 
    lineItems.put("elements", elements); 
    Map<String, Map<String, List<Map<String, String>>>> order = new HashMap<>(); 
    order.put("lineItems", lineItems); 
    List<Map<String, Map<String, List<Map<String, String>>>>> orders = new ArrayList<>(); 
    orders.add(order); 

    System.out.println(lineExists(Optional.of(orders), line1)); 
    System.out.println(lineExists(Optional.of(orders), line2)); 
} 
+0

Hallo @Tomasz, vielen Dank für diese gründliche Antwort, es ist wirklich interessant und hilfreich. Ich entwickle derzeit für die Google App Engine, kann also leider keine Lambda-Ausdrücke in meinem Code verwenden, aber ich möchte etwas lernen, damit ich diesen Beitrag in Zukunft noch einmal lesen werde. Danke noch einmal. – SBmore

0

Definitiv https://github.com/daveclayton/json-schema-validator

schreiben Schema untersuchen für Ihre JSON-Objekte und validieren sie gegen das Schema. Sie möchten keinen Validierungscode für jedes andere json-Objekt in jeder von Ihnen erstellten Anwendung schreiben.

Hier ist ein einfaches Beispiel zum Überprüfen einer Nachricht String gegen eine Schema-Zeichenfolge (sobald Sie ein Schema natürlich erstellt haben).

public void validate(String schema, String msg) { 
     JsonNode schemaNode = JsonLoader.fromString(schema); 
     JsonNode msgNode = JsonLoader.fromString(msg); 

     JsonSchemaFactory factory = JsonSchemaFactory.byDefault(); 
     JsonSchema jsonSchema = factory.getJsonSchema(schemaNode); 

     ProcessingReport report = jsonSchema.validate(msgNode); 
     if (!report.isSuccess()) { 
      throw new RuntimeException(report.toString()); 
     } 
} 

Es gibt mehr Sie mit dem Bericht zu tun, aber das gibt Ihnen eine Vorstellung davon, wie trivial es sein sollte jede json Nachricht an einem beliebigen Schema zu validieren, wie Sie mit xml tun würde.