guides - java standard coding rules
Deshazte de feo si las declaraciones (25)
Tengo este código feo:
if ( v > 10 ) size = 6;
if ( v > 22 ) size = 5;
if ( v > 51 ) size = 4;
if ( v > 68 ) size = 3;
if ( v > 117 ) size = 2;
if ( v > 145 ) size = 1;
return size;
¿Cómo puedo deshacerme de las declaraciones múltiples if?
¿Hay una regla matemática subyacente a esto? Si es así, debe usar eso: pero solo si proviene del dominio del problema, no solo una fórmula que se ajuste a los casos.
¿Por qué alguien no ha sugerido cambiar la declaración. Es mucho mejor que si no escalera.
public int getSize(int input)
{
int size = 0;
switch(input)
{
case 10:
size = 6;
break;
case 22:
size = 5;
break;
case 51:
size = 4;
break;
case 68:
size = 3;
break;
case 117:
size = 2;
break;
case 145:
size = 1;
break;
}
return size;
}
¿Qué tal tal enfoque?
int getSize(int v) {
int[] thresholds = {145, 117, 68, 51, 22, 10};
for (int i = 0; i < thresholds.length; i++) {
if (v > thresholds[i]) return i+1;
}
return 1;
}
Funcionalmente: (demostrado en Scala)
def getSize(v: Int): Int = {
val thresholds = Vector(145, 117, 68, 51, 22, 10)
thresholds.zipWithIndex.find(v > _._1).map(_._2).getOrElse(0) + 1
}
Aquí está mi oportunidad ...
Actualización: Corregido. La solución anterior dio respuestas incorrectas para los valores exactos (10,22,51 ...). Este por defecto es 6 para el if val <10
static int Foo(int val)
{
//6, 5, 4, 3, 2 ,1
int[] v = new int[]{10,22,51,68,117,145};
int pos = Arrays.binarySearch(v, val-1);
if ( pos < 0) pos = ~pos;
if ( pos > 0) pos --;
return 6-pos;
}
Aquí hay un montón de respuestas y sugerencias, pero honestamente no veo ninguna de ellas "más bonita" o "más elegante" que el método original.
Si tenía docenas o CIENTOS de iteraciones para verificar, entonces podría ver fácilmente cómo ir a algunos bucles, pero honestamente, por el puñado de comparaciones que tenía, seguir con el "si" y seguir adelante. No es tan feo.
Aquí hay una solución orientada a objetos, una clase llamada Mapper<S,T> que asigna valores de cualquier tipo que implementa comparable a cualquier tipo de destino.
Sintaxis:
Mapper<String, Integer> mapper = Mapper.from("a","b","c").to(1,2,3);
// Map a single value
System.out.println(mapper.map("beef")); // 2
// Map a Collection of values
System.out.println(mapper.mapAll(
Arrays.asList("apples","beef","lobster"))); // [1, 2, 3]
Código:
public class Mapper<S extends Comparable<S>, T> {
private final S[] source;
private final T[] target;
// Builder to enable from... to... syntax and
// to make Mapper immutable
public static class Builder<S2 extends Comparable<S2>> {
private final S2[] data;
private Builder(final S2[] data){
this.data = data;
}
public <T2> Mapper<S2, T2> to(final T2... target){
return new Mapper<S2, T2>(this.data, target);
}
}
private Mapper(final S[] source, final T[] target){
final S[] copy = Arrays.copyOf(source, source.length);
Arrays.sort(copy);
this.source = copy;
this.target = Arrays.copyOf(target, target.length);
}
// Factory method to get builder
public static <U extends Comparable<U>, V> Builder<U> from(final U... items){
return new Builder<U>(items);
}
// Map a collection of items
public Collection<T> mapAll(final Collection<? extends S> input){
final Collection<T> output = new ArrayList<T>(input.size());
for(final S s : input){
output.add(this.map(s));
}
return output;
}
// map a single item
public T map(final S input){
final int sourceOffset = Arrays.binarySearch(this.source, input);
return this.target[
Math.min(
this.target.length-1,
sourceOffset < 0 ? Math.abs(sourceOffset)-2:sourceOffset
)
];
}
}
Edición: finalmente se reemplazó el método map () con una versión más eficiente (y más corta). Lo sé: una versión que busca particiones aún sería más rápida para arreglos grandes, pero lo siento: soy demasiado perezosa.
Si crees que esto es demasiado exagerado, considera esto:
- Contiene un constructor que le permite crear el Asignador usando la sintaxis de varargs. Yo diría que es imprescindible para la usabilidad.
- Contiene un solo elemento y un método de mapeo de colección.
- Es inmutable y por lo tanto seguro de hilo
Claro, todas estas características podrían eliminarse fácilmente, pero el código sería menos completo, menos utilizable o menos estable.
El código original me parece bien, pero si no te importa la devolución múltiple, es posible que prefieras un enfoque más tabular:
if ( v > 145 ) return 1;
if ( v > 117 ) return 2;
if ( v > 68 ) return 3;
if ( v > 51 ) return 4;
if ( v > 22 ) return 5;
if ( v > 10 ) return 6;
return ...; // The <= 10 case isn''t handled in the original code snippet.
Vea la declaración múltiple o no discusión en la respuesta de org.life.java .
El problema más obvio con la solución de OP es la ramificación, por lo que sugeriría una regresión polinomial. Esto resultará en una buena expresión sin ramas en el formulario
size = round(k_0 + k_1 * v + k_2 * v^2 + ...)
Por supuesto, no obtendrá un resultado exacto, pero si puede tolerar alguna desviación, es una alternativa muy eficaz. Dado que el comportamiento ''dejar sin modificar'' de la función original para valores donde v<10 es imposible de modelar con un polinomio, me tomé la libertad de asumir una interpolación de retención de orden cero para esta región.
Para un polinomio de 45 grados con los siguientes coeficientes,
-9.1504e-91 1.1986e-87 -5.8366e-85 1.1130e-82 -2.8724e-81 3.3401e-78 -3.3185e-75 9.4624e-73 -1.1591e-70 4.1474e-69 3.7433e-67 2.2460e-65 -6.2386e-62 2.9843e-59 -7.7533e-57 7.7714e-55 1.1791e-52 -2.2370e-50 -4.7642e-48 3.3892e-46 3.8656e-43 -6.0030e-41 9.4243e-41 -1.9050e-36 8.3042e-34 -6.2687e-32 -1.6659e-29 3.0013e-27 1.5633e-25 -8.7156e-23 6.3913e-21 1.0435e-18 -3.0354e-16 3.8195e-14 -3.1282e-12 1.8382e-10 -8.0482e-09 2.6660e-07 -6.6944e-06 1.2605e-04 -1.7321e-03 1.6538e-02 -1.0173e-01 8.3042e-34 -6.2687e-32 -1.6659e-29 3.0013e-27 1.5633e-25 -8.7156e-23 6.3913e-21 1.0435e-18 -3.0354e-16 3.8195e-14 -3.1282e-12 1.8382e-10 -8.0482e-09 2.6660e-07 -6.6944e-06 1.2605e-04 -1.7321e-03 1.6538e-02 -1.0173e-01 3.6100e-01 -6.2117e-01 6.3657e+00
, obtienes una curva bellamente ajustada:
¡Y como puede ver, obtiene un error de 1 norma de solo 1.73 en todo el rango de 0 a 200 *!
* Los resultados para v∉[0,200] pueden variar.
En realidad, si es probable que los tamaños cambien, hacerlo en la base de datos podría ser una buena estrategia alternativa:
CREATE TABLE VSize (
LowerBound int NOT NULL CONSTRAINT PK_VSize PRIMARY KEY CLUSTERED,
Size int NOT NULL
)
INSERT VSize VALUES (10, 6)
INSERT VSize VALUES (22, 5)
INSERT VSize VALUES (51, 4)
INSERT VSize VALUES (68, 3)
INSERT VSize VALUES (117, 2)
INSERT VSize VALUES (145, 1)
Y un procedimiento almacenado o función:
CREATE PROCEDURE VSizeLookup
@V int,
@Size int OUT
AS
SELECT TOP 1 @Size = Size
FROM VSize
WHERE @V > LowerBound
ORDER BY LowerBound
Es interesante que haya muchas respuestas hermosas para una simple pregunta "fea". Me gusta más la respuesta de mfloryan, sin embargo, la empujaría más lejos eliminando la matriz codificada dentro del método. Algo como,
int getIndex(int v, int[] descArray) {
for(int i = 0; i < descArray.length; i++)
if(v > descArray[i]) return i + 1;
return 0;
}
Ahora se vuelve más flexible y puede procesar cualquier matriz dada en orden descendente y el método encontrará el índice al que pertenece el valor ''v''.
PD. Todavía no puedo comentar sobre las respuestas.
Este es mi ejemplo de código, utilizando SortedSet. Inicializa los límites una vez.
SortedSet<Integer> boundaries = new SortedSet<Integer>;
boundaries.add(10);
boundaries.add(22);
boundaries.add(51);
boundaries.add(68);
boundaries.add(117);
boundaries.add(145);
Luego, utilícelo posteriormente de esta manera para múltiples valores de v (y tamaño inicializado)
SortedSet<Integer> subset = boundaries.tailSet(v);
if( subset.size() != boundaries.size() )
size = subset.size() + 1;
La respuesta obvia es usar Groovy:
def size = { v -> [145,117,68,51,22,10].inject(1) { s, t -> v > t ? s : s + 1 } }
Los forros de una siempre son mejores. Devuelve 7 para el caso indefinido donde v <= 10.
Mi capacidad para comentar aún no está activada, espero que nadie diga "con razón" según mi respuesta ...
Detectar el código feo podría / debería definirse como tratar de lograr:
- Legibilidad (OK, indicando lo obvio - quizás redundante a la pregunta)
- Rendimiento: en el mejor de los casos es óptimo, en el peor, no es una gran pérdida.
- Pragmatismo: no está muy lejos de la forma en que la mayoría de las personas hace las cosas, dado un problema común que no necesita una solución elegante o única, cambiarlo más adelante debe ser un esfuerzo natural, no necesita mucho recuerdo.
En mi opinión, la respuesta dada por org.life.java fue la más bonita y extremadamente fácil de leer. También me gustó el orden en que se escribieron las condiciones, por razones de lectura y rendimiento.
Al revisar todos los comentarios sobre este tema, en el momento de mi redacción, parece que solo org.life.java planteó el problema del rendimiento (y quizás mfloryan, también, afirmando que algo sería "más largo"). Ciertamente, en la mayoría de las situaciones, y dado este ejemplo, no debería soportar una desaceleración notable como lo escriba.
Sin embargo, al anidar sus condiciones y ordenarlas de manera óptima puede mejorar el rendimiento [vale la pena, especialmente si esto fuera un bucle].
Dicho todo lo anterior, las condiciones de anidamiento y ordenamiento (que son más complejas que su ejemplo) provocadas por la determinación de lograr la ejecución más rápida posible a menudo producen un código menos legible, y un código que es más difícil de cambiar. Me refiero nuevamente al # 3, el pragmatismo ... equilibrando las necesidades.
Otra variación más (menos pronunciada que la respuesta de George )
//int v = 9;
int[] arr = {145, 117, 68, 51, 22, 10};
int size = 7; for(;7 - size < arr.length && v - arr[size - 2] > 0; size--) {};
return size;
Podrías reescribirlo en código ARM. Es solo 7 ciclos en el peor de los casos y un delgado de 164 bytes. Espero que ayude. (nota: esto no está probado)
; On entry
; r0 - undefined
; r1 - value to test
; lr - return address
; On exit
; r0 - new value or preserved
; r1 - corrupted
;
wtf
SUBS r1, r1, #10
MOVLE pc, lr
CMP r1, #135
MOVGT r0, #1
ADRLE r0, lut
LDRLEB r0, [r0, r1]
MOV pc, lr
;
; Look-up-table
lut
DCB 0 ; padding
DCB 6 ; r1 = 11 on entry
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 6
DCB 5 ; r1 = 23 on entry
DCB 5
...
ALIGN
Si realmente desea la solución de tiempo de complejidad de O grande más rápida para esta respuesta en particular, esta es una búsqueda constante.
final int minBoundary = 10;
final int maxBoundary = 145;
final int maxSize = 6;
Vector<Integer> index = new Vector<Integer>(maxBoundary);
// run through once and set the values in your index
después
if( v > minBoundary )
{
size = (v > maxBoundary ) ? maxSize : index[v];
}
Lo que estamos haciendo aquí es marcar todos los resultados posibles de v dentro del rango y dónde caen, y luego solo tenemos que probar las condiciones de contorno.
El problema con esto es que utiliza más memoria y, por supuesto, si maxBoundary es mucho más grande, será muy ineficiente en cuanto al espacio (además de demorar más tiempo en inicializarse).
Esta puede ser a veces la mejor solución para la situación.
Solo para completar, permítame sugerirle que puede configurar una TAMAÑOS de matriz con 145 elementos para que la respuesta se pueda devolver directamente como TAMAÑOS [v]. Perdóname por no escribir todo el asunto. Tendrías que asegurarte de que v estuviera dentro del rango, por supuesto.
La única razón que se me ocurra para hacerlo de esa manera sería si creara la matriz una vez y la usara miles de veces en una aplicación que tenía que ser realmente rápida. Lo menciono como un ejemplo de un compromiso entre la memoria y la velocidad (no fue el problema que alguna vez fue), y también entre el tiempo de configuración y la velocidad.
Tengo una versión más para ti. Realmente no creo que sea la mejor porque agrega una complejidad innecesaria en el nombre de "rendimiento" cuando estoy 100% seguro de que esta función nunca será un abultamiento del rendimiento (a menos que alguien esté calculando el tamaño en un bucle ajustado un millón de veces) ...).
Pero lo presento simplemente porque pensé que realizar una búsqueda binaria codificada es algo interesante . No parece muy binario-y porque no hay suficientes elementos para profundizar, pero tiene la virtud de que devuelve un resultado en no más de 3 pruebas en lugar de 6 como en la publicación original. Las declaraciones de devolución también están ordenadas por tamaño, lo que ayudaría a la comprensión y / o modificación.
if (v > 68) {
if (v > 145) {
return 1
} else if (v > 117) {
return 2;
} else {
return 3;
}
} else {
if (v > 51) {
return 4;
} else if (v > 22) {
return 5;
} else {
return 6;
}
}
Usando la API de NavigableMap :
NavigableMap<Integer, Integer> s = new TreeMap<Integer, Integer>();
s.put(10, 6);
s.put(22, 5);
s.put(51, 4);
s.put(68, 3);
s.put(117, 2);
s.put(145, 1);
return s.lowerEntry(v).getValue();
if ( v > 145 ) size = 1;
else if ( v > 117 ) size = 2;
else if ( v > 68 ) size = 3;
else if ( v > 51 ) size = 4;
else if ( v > 22 ) size = 5;
else if ( v > 10 ) size = 6;
return size;
Esto es mejor para tu caso.
Opcionalmente, debes elegir Switch Case siempre que sea posible.
Update: si ha analizado el valor de ''v'', generalmente reside en un rango inferior (<10) en la mayoría de los casos de lo que puede agregar.
if(v < 10) size = SOME_DEFAULT_VALUE;
else if ( v > 145 ) size = 1;
else if ( v > 117 ) size = 2;
else if ( v > 68 ) size = 3;
else if ( v > 51 ) size = 4;
else if ( v > 22 ) size = 5;
else if ( v > 10 ) size = 6;
further : también puede alterar la secuencia de la condición, de acuerdo con su análisis. Si sabe que la mayoría de los valores son menores que 10 y luego, en segundo lugar, la mayoría de los valores se encuentran entre 68-117, puede alterar la secuencia de condición en consecuencia.
Ediciones:
if(v < 10) return SOME_DEFAULT_VALUE;
else if ( v > 145 ) return 1;
else if ( v > 117 ) return 2;
else if ( v > 68 ) return 3;
else if ( v > 51 ) return 4;
else if ( v > 22 ) return 5;
else if ( v > 10 ) return 6;
int[] arr = new int[] {145, 117, 68, 51, 22, 10};
for(int index = 0; index < arr.length; index++)
{
if(v > arr[index]) return 1 + index;
}
return defaultValue;
return (v-173) / -27;
return v > 145 ? 1
: v > 117 ? 2
: v > 68 ? 3
: v > 51 ? 4
: v > 22 ? 5
: v > 10 ? 6
: "put inital size value here";
7 - (x>10 + x>22 + x>51 + x>68 + x>117 + x>145)
donde 7 es el valor predeterminado ( x <= 10 ).
Edit: Inicialmente no me di cuenta de que esta pregunta es sobre Java. Esta expresión no es válida en Java, pero es válida en C / C ++. Dejaré la respuesta, ya que algunos usuarios la encontraron útil.
if (v <= 10)
return size;
else {
size = 1;
if (v > 145)
return size;
else if (v > 117)
return ++size;
else if (v > 68)
return (size+2);
else if (v > 51)
return (size+3);
else if (v > 22)
return (size+4);
else if (v > 10)
return (size+5);
}
Esto ejecutará las instrucciones if necesarias solamente.