¿Debería definirse una constante de cadena si solo se va a usar una vez?

24

Estamos implementando un adaptador para Jaxen (una biblioteca XPath para Java) que nos permite usar XPath para acceder al modelo de datos de nuestra aplicación.

Esto se realiza mediante la implementación de clases que asignan cadenas (que nos pasó de Jaxen) a elementos de nuestro modelo de datos. Estimamos que necesitaremos alrededor de 100 clases con más de 1000 comparaciones de cadenas en total.

Creo que la mejor manera de hacer esto es simple si / otras declaraciones con las cadenas escritas directamente en el código, en lugar de definir cada cadena como una constante. Por ejemplo:

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

Sin embargo, siempre me enseñaron que no debemos incrustar valores de cadena directamente en el código, sino crear constantes de cadena en su lugar. Eso se vería así:

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

En este caso, creo que es una mala idea. La constante solo se usará una vez, en el getNode()método. Usar las cadenas directamente es tan fácil de leer y entender como usar constantes, y nos ahorra escribir al menos mil líneas de código.

Entonces, ¿hay alguna razón para definir constantes de cadena para un solo uso? ¿O es aceptable usar cadenas directamente?


PD. Antes de que alguien sugiera usar enumeraciones, creamos un prototipo de eso, pero la conversión de enumeración es 15 veces más lenta que la simple comparación de cadenas, por lo que no se está considerando.


Conclusión: las respuestas a continuación ampliaron el alcance de esta pregunta más allá de las constantes de cadena, por lo que tengo dos conclusiones:

  • Probablemente esté bien usar las cadenas directamente en lugar de las constantes de cadena en este escenario, pero
  • Hay formas de evitar el uso de cadenas, lo que podría ser mejor.

Así que voy a probar la técnica de envoltura que evita las cadenas por completo. Desafortunadamente, no podemos usar la instrucción de cambio de cadena porque todavía no estamos en Java 7. Sin embargo, en última instancia, creo que la mejor respuesta para nosotros es probar cada técnica y evaluar su rendimiento. La realidad es que si una técnica es claramente más rápida, entonces probablemente la elegiremos independientemente de su belleza o adhesión a la convención.

gutch
fuente
3
No planeas teclear manualmente un 1000 si las declaraciones lo hacen?
JeffO
1
Me parece muy triste lo desagradable que puede ser algo tan simple en algunos idiomas ...
Jon Purdy
55
Java 7 permite cadenas como switchetiquetas. Use un interruptor en lugar de ifcascadas.
Restablece a Monica - M. Schröder el
3
¡la conversión de enumeración es 15 veces más lenta si convierte una cadena a su valor de enumeración! Pase enum directamente y compárelo con otro valor enum del mismo tipo.
Neil
2
Los olores como HashMap pueden ser una solución.
MarioDS

Respuestas:

5

Prueba esto. La reflexión inicial es ciertamente costosa, pero si la va a usar muchas veces, lo cual creo que lo hará, esta es sin duda una mejor solución de lo que está proponiendo. No me gusta usar la reflexión, pero me encuentro usándola cuando no me gusta la alternativa a la reflexión. Creo que esto le ahorrará mucho dolor de cabeza a su equipo, pero debe pasar el nombre del método (en minúsculas).

En otras palabras, en lugar de pasar "nombre", pasaría "nombre completo" porque el nombre del método get es "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;
}

Si necesita acceder a los datos contenidos en los miembros de contacto, puede considerar crear una clase de contenedor para contacto que tenga todos los métodos para acceder a la información requerida. Esto también sería útil para garantizar que los nombres de los campos de acceso siempre serán los mismos (es decir, si la clase contenedora tiene getFullName () y llama con el nombre completo, siempre funcionará incluso si se ha cambiado el nombre del contacto getFullName (). causaría un error de compilación antes de permitirle hacer eso).

public class ContactWrapper {
    private Contact contact;

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

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

Esta solución me ha salvado varias veces, es decir, cuando quería tener una única representación de datos para usar en tablas de datos jsf y cuando esos datos debían exportarse a un informe usando jasper (que en mi experiencia no maneja bien los complicados accesores de objetos) .

Neil
fuente
Me gusta la idea de un objeto contenedor con los métodos llamados via .invoke(), porque elimina completamente las constantes de cadena. No estoy tan interesado en la reflexión en tiempo de ejecución para configurar el mapa, aunque tal vez ejecutar getMethodMapping()en un staticbloque estaría bien para que suceda al inicio en lugar de una vez que el sistema se está ejecutando.
Gutch
@gutch, el patrón de envoltura es uno que uso con frecuencia, ya que tiende a resolver muchos problemas relacionados con la interfaz / controlador. La interfaz siempre puede usar el contenedor y estar contento con él, mientras tanto, el controlador puede girarse al revés. Todo lo que necesita saber es qué datos desea que estén disponibles en la interfaz. Y nuevamente, digo para enfatizar, normalmente no me gusta la reflexión, pero si es una aplicación web, es completamente aceptable si lo haces en el inicio ya que el cliente no verá nada de ese tiempo de espera.
Neil
@Neil ¿Por qué no utilizar BeanUtils de Apache commons? También es compatible con objetos incrustados. Puede pasar por una estructura de datos completa obj.attrA.attrB.attrN y tiene muchas otras posibilidades :-)
Laiv
En lugar de mapeos con Maps, iría por @Annotations. Algo como lo hace JPA. Para definir mi propia Anotación para asignar entradas del controlador (cadena) con un atributo o captador específico. Trabajar con Annotation es bastante fácil y está disponible desde Java 1.6 (creo)
Laiv
5

Si es posible, use Java 7, que le permite usar cadenas en las switchdeclaraciones.

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

No he medido, pero creo que las declaraciones de cambio se compilan en una tabla de salto en lugar de una larga lista de comparaciones. Esto debería ser aún más rápido.

Con respecto a su pregunta real: si solo la usa una vez, no necesita convertirla en una constante. Sin embargo, considere que una constante puede documentarse y aparece en Javadoc. Esto puede ser importante para valores de cadena no triviales.


fuente
2
Sobre la mesa de salto. El conmutador de cadena se reemplaza por conmutadores, primero se basa en el código hash (la igualdad se verifica para todas las constantes con el mismo código hash) y selecciona el índice de rama, los segundos conmutadores en el índice de rama y selecciona el código de rama original. La última es claramente adecuada para una tabla de ramificación, la primera no se debe a la distribución de la función hash. Por lo tanto, cualquier ventaja de rendimiento se debe probablemente a la realización basada en hash.
scarfridge
Un muy buen punto; Si se realiza bien podría valer la pena pasar a Java 7 sólo para esto ...
Gutch
4

Si va a mantener esto (realice algún tipo de cambio no trivial), podría considerar usar algún tipo de generación de código basada en anotaciones (tal vez a través de CGLib ) o incluso solo un script que escriba todo el código para usted. Imagine la cantidad de errores tipográficos y errores que podrían aparecer con el enfoque que está considerando ...

Steven Schlansker
fuente
Consideramos anotar los métodos existentes, pero algunas asignaciones atraviesan múltiples objetos (por ejemplo, la asignación de "país" object.getAddress().getCountry()) que es difícil de representar con anotaciones. Las comparaciones de cadenas if / else no son bonitas, pero son rápidas, flexibles, fáciles de entender y fáciles de probar.
Gutch
1
Tienes razón sobre el potencial de errores tipográficos y errores; Mi única defensa son las pruebas unitarias. Por supuesto que los medios aún más código ...
Gutch
2

Todavía usaría constantes definidas en la parte superior de sus clases. Hace que su código sea más fácil de mantener, ya que es más fácil ver qué se puede cambiar más adelante (si es necesario). Por ejemplo, "first_name"podría convertirse "firstName"en algún momento posterior.

Bernardo
fuente
Sin embargo, estoy de acuerdo, si este código se generará automáticamente y las constantes no se usan en otro lugar, entonces no importa (el OP dice que necesitan hacer esto en 100 clases).
NoChance
55
Simplemente no veo el ángulo de "mantenibilidad" aquí cambia "first_name" a "givenName" una vez en un lugar en cualquier caso. Sin embargo, en el caso de las constantes con nombre, ahora queda una variable desordenada "first_name" que se refiere a una cadena "givenName", por lo que probablemente quiera cambiar eso también, así que ahora tiene tres cambios en dos lugares
James Anderson el
1
Con el IDE correcto, estos cambios son triviales. Lo que estoy defendiendo es que es más evidente dónde hacer estos cambios porque te has tomado el tiempo para declarar constantes en la parte superior de la clase y no tienes que leer el resto del código de la clase para poder hacer estos cambios
Bernard
Pero cuando está leyendo la declaración if, debe regresar y verificar que la constante contiene la cadena que cree que contiene, nada guardado aquí.
James Anderson el
1
Quizás, pero es por eso que nombro bien mis constantes.
Bernard
1

Si su nomenclatura es coherente (también "some_whatever"se asigna a getSomeWhatever()), puede usar la reflexión para determinar y ejecutar el método get.

Scarfridge
fuente
Mejor getSome_whatever (). Podría estar rompiendo el caso de los camellos, pero es mucho más importante asegurarse de que la reflexión funcione. Además, tiene la ventaja adicional de que te hace decir: "¿Por qué diablos lo hicimos de esa manera ... oh, espera ... ¡Espera! ¡George no cambie el nombre de ese método!"
Neil
0

Supongo que el procesamiento de anotaciones podría ser la solución, incluso sin anotaciones. Es lo que puede generar todo el código aburrido para usted. La desventaja es que obtendrá N clases generadas para N clases de modelos. Tampoco puede agregar nada a una clase existente, pero escribir algo como

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

una vez por clase no debería ser un problema. Alternativamente, podrías escribir algo como

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

en una superclase común


Puede usar la reflexión en lugar del procesamiento de anotaciones para la generación de código. La desventaja es que necesita su código para compilar antes de poder usar la reflexión sobre él. Esto significa que no puede confiar en el código generado en sus clases de modelo, a menos que genere algunos stubs.


También consideraría el uso directo de la reflexión. Claro, la reflexión es lenta, pero ¿por qué es lenta? Es porque tiene que hacer todo lo que necesita hacer, por ejemplo, activar el nombre del campo.

maaartinus
fuente