Czy należy zdefiniować stałą ciągu, jeśli będzie ona używana tylko raz?

24

Wdrażamy adapter dla Jaxen (biblioteka XPath dla Java), który pozwala nam używać XPath do uzyskiwania dostępu do modelu danych naszej aplikacji.

Odbywa się to poprzez implementację klas, które mapują ciągi znaków (przekazane nam z Jaxen) na elementy naszego modelu danych. Szacujemy, że potrzebujemy około 100 klas z łącznie ponad 1000 ciągów porównań.

Myślę, że najlepszym sposobem na zrobienie tego jest prosta instrukcja if / else z ciągami zapisanymi bezpośrednio w kodzie - zamiast definiowania każdego z nich jako stałej. Na przykład:

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();
    ...

Jednak zawsze uczono mnie, że nie powinniśmy osadzać wartości ciągów bezpośrednio w kodzie, ale zamiast tego tworzyć stałe ciągów. To by wyglądało mniej więcej tak:

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();
    ...

W tym przypadku myślę, że to zły pomysł. Stała będzie kiedykolwiek używana w getNode()metodzie tylko raz . Bezpośrednie korzystanie z ciągów jest tak samo łatwe do odczytania i zrozumienia, jak używanie stałych, i oszczędza nam pisania co najmniej tysiąca wierszy kodu.

Czy jest więc jakiś powód, aby definiować stałe ciągów dla pojedynczego użycia? Czy też dopuszczalne jest używanie ciągów bezpośrednio?


PS. Zanim ktokolwiek zasugeruje użycie zamiast tego wyliczeń, prototypowaliśmy to, ale konwersja wyliczenia jest 15 razy wolniejsza niż zwykłe porównywanie ciągów, więc nie jest brana pod uwagę.


Wniosek: poniższe odpowiedzi rozszerzyły zakres tego pytania poza zwykłe ciągi znaków, więc mam dwa wnioski:

  • Prawdopodobnie w tym scenariuszu można używać bezpośrednio łańcuchów zamiast stałych ciągów, ale
  • Istnieją sposoby na uniknięcie używania ciągów, co może być lepsze.

Więc wypróbuję technikę owijania, która całkowicie unika ciągów. Niestety nie możemy użyć instrukcji switch string, ponieważ nie korzystamy jeszcze z Java 7. Ostatecznie jednak uważam, że najlepszą odpowiedzią dla nas jest wypróbowanie każdej techniki i ocena jej wydajności. Rzeczywistość jest taka, że ​​jeśli jedna technika jest wyraźnie szybsza, prawdopodobnie wybierzemy ją bez względu na jej piękno lub przestrzeganie konwencji.

gutch
źródło
3
Nie planujesz ręcznie wpisywać 1000 na klawiaturze, jeśli tak?
JeffO
1
Uważam, że tak smutny jak nieprzyjemny coś to prosta może być w niektórych językach ...
Jon Purdy
5
Java 7 dopuszcza ciągi jako switchetykiety. Użyj przełącznika zamiast ifkaskad.
Przywróć Monikę - M. Schröder
3
konwersja wyliczenia jest 15 razy wolniejsza, jeśli zamienisz ciąg znaków na jego wartość wyliczenia! Przekaż wyliczenie bezpośrednio i porównaj z inną wartością wyliczenia tego samego typu!
Neil
2
Zapachy takie jak HashMap mogą być rozwiązaniem.
MarioDS

Odpowiedzi:

5

Spróbuj tego. Wstępna refleksja jest z pewnością droga, ale jeśli zamierzasz jej używać wiele razy, co, jak sądzę, będziesz, z pewnością jest to lepsze rozwiązanie tego, co proponujesz. Nie lubię używać odbicia, ale używam go, gdy nie lubię alternatywy dla odbicia. Myślę, że to zaoszczędzi Twojemu zespołowi dużo bólu głowy, ale musisz podać nazwę metody (małymi literami).

Innymi słowy, zamiast przekazać „name”, należy przekazać „fullname”, ponieważ nazwa metody get to „getFullName ()”.

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;
}

Jeśli potrzebujesz dostępu do danych zawartych w kontaktach, możesz rozważyć zbudowanie klasy opakowania dla kontaktu, która ma wszystkie metody dostępu do wymaganych informacji. Przydałoby się to również do zagwarantowania, że ​​nazwy pól dostępu pozostaną zawsze takie same (tj. Jeśli klasa opakowania ma getFullName () i wywołujesz z pełną nazwą, zawsze będzie działać, nawet jeśli nazwa getFullName () kontaktu zostanie zmieniona - to spowodowałoby błąd kompilacji, zanim pozwoli ci to zrobić).

public class ContactWrapper {
    private Contact contact;

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

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

To rozwiązanie uratowało mnie kilka razy, a mianowicie gdy chciałem mieć jedną reprezentację danych do użycia w plikach danych jsf i kiedy dane te musiały zostać wyeksportowane do raportu za pomocą jaspera (który z mojego doświadczenia nie radzi sobie dobrze ze skomplikowanymi akcesoriami do obiektów) .

Neil
źródło
Podoba mi się pomysł opakowania obiektu za pomocą metod wywoływanych przez via .invoke(), ponieważ całkowicie usuwa stałe ciągów znaków. Nie przepadam za refleksją środowiska uruchomieniowego w celu skonfigurowania mapy, chociaż może wykonanie getMethodMapping()w staticbloku byłoby OK, więc dzieje się to przy starcie, a nie po uruchomieniu systemu.
gutch
@gutch, wzór opakowania jest tym, którego często używam, ponieważ ma on tendencję do rozwiązywania wielu problemów związanych z interfejsem / kontrolerem. Interfejs zawsze może korzystać z opakowania i być z niego zadowolonym, a kontroler można w międzyczasie wywrócić na lewą stronę. Wszystko, co musisz wiedzieć, to jakie dane mają być dostępne w interfejsie. I znowu, podkreślam, że zwykle nie lubię refleksji, ale jeśli jest to aplikacja internetowa, jest to całkowicie akceptowalne, jeśli robisz to przy starcie, ponieważ klient nie zobaczy tego czasu oczekiwania.
Neil
@Neil Dlaczego nie korzystać z BeanUtils z Apache commons? Obsługuje również obiekty osadzone. Możesz przejść przez całą strukturę danych obj.attrA.attrB.attrN i ma wiele innych możliwości :-)
Laiv
Zamiast mapowań w Mapach wybrałbym @ Adnotacje. Coś jak JPA. Aby zdefiniować własną adnotację do mapowania wpisów kontrolera (ciąg znaków) za pomocą określonego atrybutu lub gettera. Praca z Adnotacją jest dość łatwa i jest dostępna w Javie 1.6 (tak myślę)
Laiv
5

Jeśli to w ogóle możliwe, użyj Java 7, który pozwala na użycie ciągów w switchinstrukcjach.

From 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);
        }
    }
}

Nie mierzyłem, ale uważam, że instrukcje przełączania kompilują się do tabeli skoków zamiast długiej listy porównań. To powinno być jeszcze szybsze.

Odnośnie twojego rzeczywistego pytania: jeśli użyjesz go tylko raz , nie musisz przekształcać go w stałą. Zastanów się jednak, że stałą można udokumentować i wyświetlić w Javadoc. Może to być ważne w przypadku nietrywialnych wartości ciągu.


źródło
2
Dotyczące stołu skoków. Przełącznik String zastępuje się przełącznikami, najpierw oparty jest na haszyszu (równość jest sprawdzana dla wszystkich stałych o tym samym haszu) i wybiera indeks gałęzi, drugi włącza indeks gałęzi i wybiera oryginalny kod gałęzi. Ten drugi jest wyraźnie odpowiedni dla tabeli rozgałęzień, pierwszy nie wynika z dystrybucji funkcji skrótu. Tak więc każda przewaga wydajności prawdopodobnie wynika z realizacji opartej na haszowaniu.
scarfridge
To bardzo dobra uwaga; jeżeli wykonuje dobrze może warto poruszający się Java 7 tylko za to ...
Gutch
4

Jeśli zamierzasz to utrzymać (dokonać jakichkolwiek nietrywialnych zmian), mógłbym rozważyć użycie generowania kodu opartego na adnotacjach (być może za pomocą CGLib ), a nawet po prostu skryptu, który napisze cały kod za Ciebie. Wyobraź sobie liczbę literówek i błędów, które mogą wkraść się w rozważane podejście ...

Steven Schlansker
źródło
Zastanawialiśmy się nad opisaniem istniejących metod, ale niektóre mapowania przechodzą przez wiele obiektów (na przykład mapowanie „kraju” object.getAddress().getCountry()), co jest trudne do przedstawienia za pomocą adnotacji. Porównywania ciągów if / else nie są ładne, ale są szybkie, elastyczne, łatwe do zrozumienia i łatwe do testowania jednostkowego.
gutch
1
Masz rację co do możliwości literówek i błędów; moją jedyną obroną są testy jednostkowe. Oczywiście oznacza to jeszcze więcej kodu ...
gutch
2

Nadal używałbym stałych zdefiniowanych na szczycie twoich klas. To sprawia, że ​​kod jest łatwiejszy w utrzymaniu, ponieważ łatwiej jest zobaczyć, co można zmienić później (w razie potrzeby). Na przykład "first_name"może stać "firstName"się później.

Bernard
źródło
Zgadzam się jednak, że jeśli ten kod zostanie wygenerowany automatycznie, a stałe nie zostaną użyte w innym miejscu, to nie ma to znaczenia (OP mówi, że muszą to zrobić w 100 klasach).
NoChance
5
Po prostu nie widzę tutaj kąta „łatwości konserwacji”, w obu przypadkach zmieniasz raz „imię” na „imię” raz w jednym miejscu. Jednak w przypadku nazwanych stałych pozostaje ci niechlujna zmienna „imię”, która odnosi się do ciąg „podanoNazwa”, więc prawdopodobnie również chcesz to zmienić, więc teraz masz trzy zmiany w dwóch miejscach
James Anderson
1
Przy odpowiednim IDE zmiany są banalne. To, co zalecam, to to, że bardziej oczywiste jest, gdzie wprowadzić te zmiany, ponieważ poświęciłeś czas na zadeklarowanie stałych na szczycie klasy i nie musisz czytać reszty kodu w klasie, aby dokonaj tych zmian.
Bernard,
Ale kiedy czytasz instrukcję if, musisz wrócić i sprawdzić, czy stała zawiera ciąg, który Twoim zdaniem zawiera - nic tu nie zapisano.
James Anderson
1
Być może, ale dlatego dobrze nazywam moje stałe.
Bernard,
1

Jeśli twoje nazewnictwo jest spójne (aka "some_whatever"jest zawsze mapowane getSomeWhatever()), możesz użyć refleksji do ustalenia i wykonania metody get.

szalik
źródło
Lepiej getSome_whokolwiek (). Może to być łamanie obudowy wielbłąda, ale o wiele ważniejsze jest upewnienie się, że odbicie działa. Dodatkowo ma tę dodatkową zaletę, że sprawia, że ​​mówisz: „Dlaczego, do cholery, zrobiliśmy to w ten sposób… oh, czekaj… Poczekaj! George nie zmienia nazwy tej metody!”
Neil
0

Myślę, że przetwarzanie adnotacji może być rozwiązaniem, nawet bez adnotacji. Jest to rzecz, która może wygenerować dla Ciebie cały nudny kod. Minusem jest to, że otrzymasz N generowanych klas dla N klas modeli. Nie możesz również dodać niczego do istniejącej klasy, ale pisać coś w stylu

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

raz na zajęcia nie powinno stanowić problemu. Możesz też napisać coś takiego

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

we wspólnej nadklasie.


Do generowania kodu można użyć refleksji zamiast przetwarzania adnotacji. Minusem jest to, że musisz skompilować swój kod, zanim będziesz mógł użyć refleksji nad nim. Oznacza to, że nie możesz polegać na generowanym kodzie w swoich klasach modeli, chyba że wygenerujesz niektóre kody pośredniczące.


Rozważę także bezpośrednie wykorzystanie refleksji. Jasne, odbicie jest powolne, ale dlaczego jest powolne? To dlatego, że musi robić wszystko, co musisz zrobić, np. Włączyć nazwę pola.

maaartinus
źródło