¿Olor del código? - las variables de ajuste con + - 1

que acabo de escribir este método:¿Olor del código? - las variables de ajuste con + - 1

private String getNameOfFileFrom(String path) 
{ 
    int indexOfLastSeparator = path.lastIndexOf('/'); 
    if (indexOfLastSeparator > -1) 
    { 
     return path.substring(indexOfLastSeparator + 1); 
    } 
    else 
    { 
     return path; 
    } 
}

La línea que me molesta es:

return path.substring(indexOfLastSeparator + 1);

¿Eso es una mala práctica para modificar la expresión en línea así? Las sugerencias sobre cómo refactorizar para aumentar la legibilidad serían bienvenidas.

---- Editar ---- Aceptar actualización después de los comentarios. Gracias a todos por responder :) No estoy buscando realmente cambiar el valor de la variable en absoluto solo la expresión en que se usa.

Otro publicado me sugirió que podría partir esa parte de la expresión como en el segundo fragmento de código a continuación. Mejor/peor/sin diferencia? :) Estoy empezando a sospechar que estoy siendo excesivamente prudente aquí.

return path.substring(indexOfLastSeparator + 1);

int indexOfFirstCharInFileName = indexOfLastSeparator + 1; 
return path.substring(indexOfFirstCharInFileName);

Fuente

2009-01-20 willcodejavaforfood

No creo que haya un problema con ++ variable. Eliminé mi respuesta porque su pregunta es independiente del idioma. Pero si estás escribiendoNET, debe usar la clase System.IO.Path para su lógica de análisis de ruta de archivo. – Will

El operador ++ no es relevante aquí, ya que la variable en sí misma no se está modificando. –

Respondió a alguien que eliminó su comentario ... – Will

Si quieres llegar hasta el final, me gustaría hacer algo como esto:

int indexOfLastSeparator = path.lastIndexOf('/'); 
    if (indexOfLastSeparator > -1) 
    { 
      int indexOfFirstCharInFilename = indexOfLastSeparator + 1; 
      return path.substring(indexOfFirstCharInFilename); 
    } 
    else 
    { 
      return path; 
    }

Esto no es sin embargo con un buen rendimiento, ya que se crea una nueva variable, pero es muy fácil de leer en la forma en que cree que lo espera

Personalmente preferiría algo así como John Nolan sugirió:

return path.substring(++indexOfLastSeparator);

No es que hablador como usted lo quiere, pero incluso un principiante de programación tiene la idea de si leer la descripción del método subcadena.

editar: Tenga en cuenta que el uso de "/" en la verificación de la ruta no es independiente de la plataforma. Use una función lib para obtener el separador dependiente del sistema si su código debe ejecutarse en diferentes plataformas.

Fuente

2009-01-20 14:28:34 bl4ckb0l7

Realmente no creo que el rendimiento sea un gran problema aquí. Es probable que el optimizador optimice la salida de la variable. Pero discutiría si esto es más legible en el caso de las OP publicaciones. Existe tal cosa como que el código es excesivamente detallado. –

"return path.substring (++ indexOfLastSeparator);" - Además, recomendaría contra esta línea. * No * hace lo mismo que lo que publicó el OP. En este caso, no importaría. Pero si los OP modificaran el código para usar esa var más adelante, podrían tener algunos errores difíciles de depurar. –

indexOfFirstcharInFileName es algo así como lo que estaba pensando que está bien, pero parece llevar todo un poco demasiado lejos, ¿no? Buen punto sobre el nombre del método también. – willcodejavaforfood

En este caso, sólo puede hacer:

private String getNameOfFileFrom(String path) 
{ 
     return path.substring(path.lastIndexOf('/') + 1); 
}

Al menos, tanto en Java y .NET, x.substring(0) devolverá una cadena igual. En Java devolverá la misma referencia, no estoy seguro acerca de .NET.

En general, hay muchas ocasiones en las que tiene sentido sumar o restar una. Realmente no creo que sea un olor a código.

Fuente

2009-01-20 14:23:25

Si está utilizando .NET vistazo a System.Io.Path.Combine(string,string) que está utilizando para combinar 2 cadenas sin comprobar y/o eliminación de terminación barras diagonales

o puede utilizar string.Remove(string.LastIndexOf("/"));

Fuente

2009-01-20 14:23:30 abatishchev

Si la segunda parte de la ruta está rooteada ("\ this \ path \ is \ rooted"), Combine se comporta de una manera que normalmente no esperaría. – Will

no veo por qué eso sería ser una mala práctica. Tal vez puedas aumentar el nombre de la variable antes de esa línea y usarla con el nuevo valor.

Pero además de eso, no veo un problema

Fuente

2009-01-20 14:23:32 Megacan

no tengo un problema con lo que está haciendo allí, pero creo que se podría utilizar un comentario que quede absolutamente claro que eres buscando el último/y tomando el resto de la cadena después de esa posición.

Fuente

2009-01-20 14:24:44

no está modificando la variable allí.

Para hacer eso sería

return path.substring(indexOfLastSeparator++);

return path.substring(++indexOfLastSeparator);

Fuente

2009-01-20 14:25:10

Y de esas opciones, solo la segunda proporcionaría el resultado que desea (el operador posterior al incremento devolvería la variable original). –

eso es correcto Phill. –

Lo siento, no estoy tratando de duplicar la variable, solo la expresión – willcodejavaforfood

Mientras que es fácil de leer y entender el código, no veo ningún problema con esta técnica. En realidad, podría volverse menos legible si, por ejemplo, aumentaras la variable en una línea adicional.

Fuente

2009-01-20 14:25:25 mafu

No modifica la variable en sí, solo modifica la expresión.Y eso es totalmente bien, especialmente en este caso como usted quiere que su nueva cadena que comienza en el carácter después de la posición del último separador.

Fuente

2009-01-20 14:26:20 Bombe

Probablemente podría haber mejorado la redacción de mi encabezado para mi problema – willcodejavaforfood

. De todos modos, no creo que ese código huele. Hace la intención bastante clara y eso casi nunca está mal. :) – Bombe

En realidad, en .net probablemente debería utilizar Path.GetFilename(); lugar.

Fuente

2009-01-20 14:28:47

Lo suficiente, además del punto :) – willcodejavaforfood

Se ES en GDI + o código de dibujo.

A veces un problema subyacente hace que algo sea apagado por un píxel o menos y en este punto tiene dos opciones.

solucionar el problema subyacente
Realice todas las operaciones posteriores utilizan esta mierda +1 o -1 offset.

Si se elige este último, este olor de código se ensuciará en todo el código.

Fuente

2009-01-20 14:31:11 Quibblesome

Hay dos razones para preferir escribir una constante durante un número mágico:

Si el valor cambia, refactorización puede ser una pesadilla cuando se utiliza un número mágico
números mágicos disminuyen la legibilidad.

Si está seguro de que ninguno de estos es un problema, le pido que lo haga. En el caso del código que da como ejemplo, creo que:

El desplazamiento de la trayectoria del último separador es poco probable que cambie y
no es terriblemente difícil saber lo que está pasando.

Así que digo que este es un uso válido de un número mágico. Sin embargo, también me gustaría señalar que usted no tiene que preocuparse por estas dos cuestiones más a menudo que no, por lo que en caso de duda, utilice una constante.

Fuente

2009-01-20 14:34:50

Sin embargo, 1 no es un número mágico. No podrá, bajo ninguna circunstancia puedo ver, tener que cambiar el 1 a un 3 o 45, o cualquier otro número. Recuerde que los únicos números permitidos en el código son 0, 1 y posiblemente 2. – Kibbee

Eso es lo que estaba tratando de decir. Intentaba hacer que el problema fuera menos una regla (solo puedes usar 0, 1 y tal vez 2) y más un análisis de costo/beneficio (puedes usar un literal, pero x sucederá). En mi opinión, las reglas son menos importantes que el razonamiento detrás de esas reglas. –

En Java path.substring (n) no modifica la ruta, ni siquiera el objeto String al que hace referencia la ruta.

Fuente

2009-01-20 14:36:05 Darron

¿Qué tiene eso que ver con la pregunta? –

¿Olor del código? - las variables de ajuste con + - 1

Respuesta

Cuestiones relacionadas