Soll eine String-Konstante definiert werden, wenn sie nur einmal verwendet wird?

24

Wir implementieren einen Adapter für Jaxen (eine XPath-Bibliothek für Java), mit dem wir über XPath auf das Datenmodell unserer Anwendung zugreifen können.

Dies geschieht durch die Implementierung von Klassen, die Zeichenfolgen (die uns von Jaxen übergeben wurden) in Elemente unseres Datenmodells abbilden. Wir schätzen, dass wir ungefähr 100 Klassen mit insgesamt über 1000 Zeichenfolgenvergleichen benötigen.

Ich denke, dass der beste Weg, dies zu tun, einfach ist, wenn / else-Anweisungen mit den Zeichenfolgen direkt in den Code geschrieben - anstatt jede Zeichenfolge als Konstante zu definieren. Beispielsweise:

public Object getNode(String name) {
    if ("name".equals(name)) {
        return contact.getFullName();
    } else if ("title".equals(name)) {
        return contact.getTitle();
    } else if ("first_name".equals(name)) {
        return contact.getFirstName();
    } else if ("last_name".equals(name)) {
        return contact.getLastName();
    ...

Es wurde mir jedoch immer beigebracht, dass wir keine Zeichenfolgenwerte direkt in Code einbetten sollten, sondern stattdessen Zeichenfolgenkonstanten erstellen sollten. Das würde ungefähr so ​​aussehen:

private static final String NAME = "name";
private static final String TITLE = "title";
private static final String FIRST_NAME = "first_name";
private static final String LAST_NAME = "last_name";

public Object getNode(String name) {
    if (NAME.equals(name)) {
        return contact.getFullName();
    } else if (TITLE.equals(name)) {
        return contact.getTitle();
    } else if (FIRST_NAME.equals(name)) {
        return contact.getFirstName();
    } else if (LAST_NAME.equals(name)) {
        return contact.getLastName();
    ...

In diesem Fall halte ich es für eine schlechte Idee. Die Konstante wird in der getNode()Methode immer nur einmal verwendet. Die direkte Verwendung der Zeichenfolgen ist genauso einfach zu lesen und zu verstehen wie die Verwendung von Konstanten und erspart uns das Schreiben von mindestens tausend Codezeilen.

Gibt es also einen Grund, String-Konstanten für eine einmalige Verwendung zu definieren? Oder ist es akzeptabel, Zeichenfolgen direkt zu verwenden?


PS. Bevor jemand vorschlägt, stattdessen Aufzählungen zu verwenden, haben wir dies als Prototyp erstellt, aber die Aufzählungskonvertierung ist 15-mal langsamer als ein einfacher Zeichenfolgenvergleich, sodass dies nicht berücksichtigt wird.


Schlussfolgerung: Die folgenden Antworten haben den Umfang dieser Frage über reine String-Konstanten hinaus erweitert, sodass ich zwei Schlussfolgerungen ziehen kann:

  • In diesem Szenario ist es wahrscheinlich in Ordnung, die Zeichenfolgen direkt anstelle von Zeichenfolgenkonstanten zu verwenden
  • Es gibt Möglichkeiten, die Verwendung von Zeichenfolgen zu vermeiden, was möglicherweise besser ist.

Also werde ich die Wrapper-Technik ausprobieren, die Strings komplett vermeidet. Leider können wir die Anweisung string switch nicht verwenden, da wir noch nicht mit Java 7 arbeiten. Letztendlich denke ich jedoch, dass die beste Antwort für uns darin besteht, jede Technik auszuprobieren und ihre Leistung zu bewerten. Die Realität ist, dass wenn eine Technik deutlich schneller ist, wir sie wahrscheinlich unabhängig von ihrer Schönheit oder Konvention wählen werden.

gutch
quelle
3
Sie haben nicht vor, 1000 manuell zu tasten, wenn Sie dies tun?
JeffO
1
Ich finde es so traurig, wie unangenehm etwas so Einfaches in manchen Sprachen sein kann ...
Jon Purdy
5
Java 7 erlaubt Strings als switchLabels. Verwenden Sie einen Schalter anstelle von ifKaskaden.
Wiedereinsetzung von Monica - M. Schröder
3
Die Enum-Konvertierung ist 15-mal langsamer, wenn Sie einen String in seinen Enum-Wert konvertieren! Übergebe Enum direkt und vergleiche mit einem anderen Enum-Wert desselben Typs!
Neil
2
Gerüche wie HashMap können eine Lösung sein.
MarioDS

Antworten:

5

Versuche dies. Das anfängliche Nachdenken ist sicherlich teuer, aber wenn Sie es, wie ich denke, viele Male verwenden werden, ist dies mit Sicherheit eine bessere Lösung für Ihre Vorschläge. Ich benutze Reflexion nicht gern, aber ich benutze sie, wenn mir die Alternative zur Reflexion nicht gefällt. Ich denke, dass dies Ihrem Team viel Kopfzerbrechen erspart, aber Sie müssen den Namen der Methode (in Kleinbuchstaben) übergeben.

Mit anderen Worten, anstatt "name" zu übergeben, würden Sie "fullname" übergeben, da der Name der get-Methode "getFullName ()" lautet.

Map<String, Method> methodMapping = null;

public Object getNode(String name) {
    Map<String, Method> methods = getMethodMapping(contact.getClass());
    return methods.get(name).invoke(contact);
}

public Map<String, Method> getMethodMapping(Class<?> contact) {
    if(methodMapping == null) {
        Map<String, Method> mapping = new HashMap<String, Method>();
        Method[] methods = contact.getDeclaredMethods();
        for(Method method : methods) {
            if(method.getParameterTypes().length() == 0) {
                if(method.getName().startsWith("get")) {
                    mapping.put(method.getName().substring(3).toLower(), method);
                } else if (method.getName().startsWith("is"))) {
                    mapping.put(method.getName().substring(2).toLower(), method);
                }
            }
        }
        methodMapping = mapping;
    }
    return methodMapping;
}

Wenn Sie auf Daten zugreifen müssen, die in Kontaktmitgliedern enthalten sind, können Sie eine Wrapper-Klasse für contact erstellen, die über alle Methoden verfügt, um auf alle erforderlichen Informationen zuzugreifen. Dies ist auch nützlich, um sicherzustellen, dass die Namen der Zugriffsfelder immer gleich bleiben (dh wenn die Wrapper-Klasse getFullName () hat und Sie mit fullname aufrufen, funktioniert dies immer, auch wenn getFullName () des Kontakts umbenannt wurde - it würde einen Kompilierungsfehler verursachen, bevor Sie das tun könnten.

public class ContactWrapper {
    private Contact contact;

    public ContactWrapper(Contact contact) {
        this.contact = contact;
    }

    public String getFullName() {
        return contact.getFullName();
    }
    ...
}

Diese Lösung hat mich mehrere Male gerettet, nämlich als ich eine einzige Datendarstellung zur Verwendung in jsf-Datentabellen haben wollte und als diese Daten mit Jasper in einen Bericht exportiert werden mussten (was meiner Erfahrung nach mit komplizierten Objektzugriffen nicht gut zurechtkommt). .

Neil
quelle
Ich mag die Idee eines Wrapper-Objekts mit den über aufgerufenen Methoden .invoke(), weil es ganz auf String-Konstanten verzichtet. Ich bin nicht so begeistert von der Laufzeitreflexion, um die Map einzurichten, obwohl die Ausführung getMethodMapping()in einem staticBlock möglicherweise in Ordnung wäre, damit sie beim Start ausgeführt wird und nicht erst, wenn das System ausgeführt wird.
Gutch
@gutch, das Wrapper-Pattern verwende ich oft, da es dazu neigt, viele Probleme im Zusammenhang mit der Schnittstelle / dem Controller zu lösen. Die Benutzeroberfläche kann den Wrapper immer verwenden und damit zufrieden sein, und der Controller kann in der Zwischenzeit umgedreht werden. Sie müssen lediglich wissen, welche Daten in der Benutzeroberfläche verfügbar sein sollen. Ich möchte noch einmal betonen, dass ich Reflexion normalerweise nicht mag, aber wenn es sich um eine Webanwendung handelt, ist es völlig akzeptabel, wenn Sie dies beim Start tun, da der Client diese Wartezeit nicht sieht.
Neil
@Neil Warum BeanUtils von Apache Commons nicht verwenden? Es werden auch eingebettete Objekte unterstützt. Sie können durch eine ganze Datenstruktur gehen obj.attrA.attrB.attrN und es hat viele andere Möglichkeiten :-)
Laiv
Anstelle von Mappings mit Maps würde ich mich für @Annotations entscheiden. So etwas wie JPA. Eigene Annotation für die Zuordnung von Controller-Einträgen (String) mit einem bestimmten Attribut oder Getter definieren. Die Arbeit mit Annotation ist recht einfach und von Java 1.6 (glaube ich)
Laiv,
5

Verwenden Sie nach Möglichkeit Java 7, mit dem Sie Strings in switchAnweisungen verwenden können.

Von http://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html

public class StringSwitchDemo {

    public static int getMonthNumber(String month) {

        int monthNumber = 0;

        if (month == null) {
            return monthNumber;
        }

        switch (month.toLowerCase()) {
            case "january":
                monthNumber = 1;
                break;
            case "february":
                monthNumber = 2;
                break;
            case "march":
                monthNumber = 3;
                break;
            case "april":
                monthNumber = 4;
                break;
            case "may":
                monthNumber = 5;
                break;
            case "june":
                monthNumber = 6;
                break;
            case "july":
                monthNumber = 7;
                break;
            case "august":
                monthNumber = 8;
                break;
            case "september":
                monthNumber = 9;
                break;
            case "october":
                monthNumber = 10;
                break;
            case "november":
                monthNumber = 11;
                break;
            case "december":
                monthNumber = 12;
                break;
            default: 
                monthNumber = 0;
                break;
        }

        return monthNumber;
    }

    public static void main(String[] args) {

        String month = "August";

        int returnedMonthNumber =
            StringSwitchDemo.getMonthNumber(month);

        if (returnedMonthNumber == 0) {
            System.out.println("Invalid month");
        } else {
            System.out.println(returnedMonthNumber);
        }
    }
}

Ich habe nicht gemessen, aber ich glaube, dass die switch-Anweisungen zu einer Sprungtabelle kompiliert werden, anstatt zu einer langen Liste von Vergleichen. Dies sollte noch schneller sein.

Zu Ihrer eigentlichen Frage: Wenn Sie sie nur einmal verwenden, müssen Sie sie nicht zu einer Konstanten machen. Beachten Sie jedoch, dass eine Konstante dokumentiert werden kann und in Javadoc angezeigt wird. Dies kann für nicht triviale Zeichenfolgenwerte wichtig sein.


quelle
2
Bezüglich der Sprungtabelle. Der String-Schalter wird durch to-Schalter ersetzt. Erstens basiert er auf dem Hashcode (die Gleichheit wird auf alle Konstanten mit demselben Hashcode überprüft) und wählt den Verzweigungsindex aus, zweitens schaltet er den Verzweigungsindex ein und wählt den ursprünglichen Verzweigungscode aus. Letzteres ist eindeutig für eine Verzweigungstabelle geeignet, erstere ist nicht auf die Verteilung der Hash-Funktion zurückzuführen. Jeder Leistungsvorteil ist wahrscheinlich auf die Hash-basierte Realisierung zurückzuführen.
scarfridge
Ein sehr guter Punkt; wenn es gut
funktioniert
4

Wenn Sie dies beibehalten möchten (und niemals irgendwelche nicht trivialen Änderungen vornehmen möchten), könnte ich in Betracht ziehen, entweder eine auf Anmerkungen basierende Codegenerierung (möglicherweise über CGLib ) oder sogar nur ein Skript zu verwenden, das den gesamten Code für Sie schreibt. Stellen Sie sich die Anzahl der Tippfehler und Fehler vor, die sich bei dem von Ihnen in Betracht gezogenen Ansatz einschleichen könnten ...

Steven Schlansker
quelle
Wir haben überlegt, vorhandene Methoden mit Anmerkungen zu versehen, aber einige Zuordnungen durchlaufen mehrere Objekte (z. B. die Zuordnung "Land" zu object.getAddress().getCountry()), die mit Anmerkungen nur schwer darstellbar sind. Die if / else-Zeichenfolgenvergleiche sind nicht besonders hübsch, aber sie sind schnell, flexibel, leicht zu verstehen und einfach zu testen.
Gutch
1
Sie haben Recht mit der Möglichkeit von Tippfehlern und Fehlern. Meine einzige Verteidigung sind Unit-Tests. Das bedeutet natürlich noch mehr Code ...
gutch
2

Ich würde immer noch Konstanten verwenden, die oben in Ihren Klassen definiert sind. Dies macht Ihren Code wartungsfreundlicher, da leichter erkennbar ist, was zu einem späteren Zeitpunkt geändert werden kann (falls erforderlich). Könnte zum Beispiel "first_name"zu "firstName"einem späteren Zeitpunkt werden.

Bernard
quelle
Ich stimme jedoch zu, wenn dieser Code automatisch generiert wird und die Konstanten nicht an anderer Stelle verwendet werden, spielt dies keine Rolle (laut OP muss dies in 100 Klassen erfolgen).
NoChance
5
Ich sehe nur nicht, dass der "Wartbarkeitswinkel" hier "Vorname" einmal an einer Stelle in "Vorname" geändert wird. Bei benannten Konstanten bleibt jedoch jetzt eine unordentliche Variable "Vorname" übrig, die sich auf "Vorname" bezieht eine Zeichenfolge "givenName", also möchten Sie das wahrscheinlich auch ändern, sodass Sie jetzt drei Änderungen an zwei Stellen haben
James Anderson
1
Mit der richtigen IDE sind diese Änderungen trivial. Was ich befürworte, ist, dass es offensichtlicher ist, wo diese Änderungen vorgenommen werden müssen, da Sie sich die Zeit genommen haben, Konstanten am Anfang der Klasse zu deklarieren und nicht den Rest des Codes in der Klasse durchzulesen, um dies zu erreichen Nehmen Sie diese Änderungen vor.
Bernard
Aber wenn Sie die if-Anweisung lesen, müssen Sie zurückgehen und prüfen, ob die Konstante die Zeichenfolge enthält, von der Sie glauben, dass sie sie enthält - hier wird nichts gespeichert.
James Anderson
1
Vielleicht, aber deshalb nenne ich meine Konstanten gut.
Bernard
1

Wenn Ihre Benennung konsistent ist (auch bekannt "some_whatever"als immer zugeordnet getSomeWhatever()), können Sie mithilfe der Reflektion die get-Methode ermitteln und ausführen.

Narbenkühlschrank
quelle
Besser getSome_whatever (). Es könnte sein, dass der Kamelkasten kaputt geht, aber es ist weitaus wichtiger, sicherzustellen, dass die Reflexion funktioniert. Außerdem hat es den zusätzlichen Vorteil, dass man sagt: "Warum zum Teufel haben wir das so gemacht? Oh, warte ... warte! George, ändere den Namen dieser Methode nicht!"
Neil
0

Ich denke, Annotation Processing könnte die Lösung sein, auch ohne Annotationen. Es ist die Sache, die den ganzen langweiligen Code für Sie erzeugen kann. Der Nachteil ist, dass Sie N generierte Klassen für N Modellklassen erhalten. Sie können einer vorhandenen Klasse auch nichts hinzufügen, sondern nur so etwas schreiben

public Object getNode(String name) {
    return SomeModelClassHelper.getNode(this, name);
}

einmal pro klasse sollte kein problem sein. Alternativ könnte man so etwas schreiben

public Object getNode(String name) {
    return getHelper(getClass()).getNode(this, name);
}

in einer gemeinsamen Oberklasse.


Sie können Reflection anstelle der Annotation-Verarbeitung für die Codegenerierung verwenden. Der Nachteil ist, dass Sie Ihren Code erst kompilieren müssen, bevor Sie ihn reflektieren können. Dies bedeutet, dass Sie sich nicht auf den generierten Code in Ihren Modellklassen verlassen können, es sei denn, Sie generieren einige Stubs.


Ich würde auch die direkte Verwendung von Reflexion in Betracht ziehen. Klar, Reflexion ist langsam, aber warum ist es langsam? Es ist, weil es alle Dinge tun muss, die Sie tun müssen, z. B. den Feldnamen einschalten.

maaartinus
quelle