¿SECAR o no SECAR? Al evitar la duplicación de código y mantener la cohesión

Question

Tengo una pregunta sobre duplicación de código y refactorización, espero que no sea demasiado general. Supongamos que tiene un fragmento de código bastante pequeño (~ 5 líneas) que es una secuencia de invocaciones de función que no es un nivel muy bajo. Este código se repite en varios lugares, por lo que probablemente sea una buena idea extraer un método aquí. Sin embargo, en este ejemplo particular, esta nueva función sufriría de baja cohesión (que se manifiesta, entre otros, por tener dificultades para encontrar un buen nombre para la función). La razón de esto es probablemente porque este código repetido es solo una parte de un algoritmo más grande, y es difícil dividirlo en pasos bien nombrados.

¿Qué sugerirías en tal escenario?

Editar:

quería mantener la cuestión en un plano general, para que más gente puede encontrar potencialmente útil, pero es evidente que lo mejor sería que lo respalde con algún ejemplo de código. El ejemplo podría no ser el mejor de todos (huele en bastantes maneras), pero espero que hace su trabajo:

class SocketAction { 

    private static class AlwaysCreateSessionLoginHandler extends LoginHandler { 
     @Override 
     protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException { 
      Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID()); 
      socketAction.registerSession(); 
      socketAction._sess.runApplication(); 
     } 
    } 

    private static class AutoConnectAnyDeviceLoginHandler extends LoginHandler { 
     @Override 
     protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException { 
      if (Server.isUserRegistered(socketAction._sess.getUserLogin())) { 
       Log.logSysInfo("Session autoconnect - acquiring list of action threads..."); 
       String[] sa = Server.getSessionList(socketAction._sess.getUserID()); 
       Log.logSysInfo("Session autoconnect - list of action threads acquired."); 
       for (int i = 0; i < sa.length; i += 7) { 
        socketAction.abandonCommThreads(); 
        Server.attachSocketToSession(sa[i + 1], socketAction._commSendThread.getSock()); 
        return; 
       } 
      } 
      Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID()); 
      socketAction.registerSession(); 
      socketAction._sess.runApplication(); 
     } 
    } 

    private static class OnlyNewSessionLoginHandler extends LoginHandler { 
     @Override 
     protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException { 
      socketAction.killOldSessionsForUser(); 
      Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID()); 
      socketAction.registerSession(); 
      socketAction._sess.runApplication(); 
     } 
    } 
} 

Answer 1

pregunta es demasiado general que decir en realidad, sino como un ejercicio:

Supongamos que lo abstrae. Piensa cuáles son las razones probables para querer cambiar la función resultante de 5 líneas. ¿Le gustaría realizar cambios que se apliquen a todos los usuarios, o terminaría teniendo que escribir una nueva función que es ligeramente diferente de la anterior, cada vez que alguien tiene razones para querer un cambio?

Si desea cambiarlo para todos los usuarios, es una abstracción viable. Dale un nombre pobre ahora, podrías pensar en uno mejor más tarde.

Si va a terminar dividiendo esta función en muchas versiones similares a medida que su código evoluciona en el futuro, probablemente no sea una abstracción viable. Aún puede escribir la función, pero es más una "función auxiliar" de ahorro de código que parte de su modelo formal del problema. Esto no es muy satisfactorio: la repetición de esta cantidad de código es un poco preocupante, porque sugiere que debería ser una abstracción viable en alguna parte.

Tal vez 4 de las 5 líneas podrían ser abstraídas, ya que son más cohesivas, y la quinta línea está colgando con ellas en este momento. Entonces podría escribir 2 nuevas funciones: una que es esta nueva abstracción, y la otra es simplemente una ayuda que llama a la nueva función y luego ejecuta la línea 5. Una de estas funciones podría tener una vida útil más larga que la otra.

Answer 2

Estuve pensando en esto últimamente y entiendo exactamente en lo que se está metiendo. Se me ocurre que esta es una abstracción de implementación más que nada, y por lo tanto es más aceptable si puede evitar cambiar una interfaz. Por ejemplo, en C++ podría extraer la función en la cpp sin tocar el encabezado. Esto en cierto modo disminuye el requisito de que la abstracción de la función esté bien formada y tenga sentido para el usuario de la clase, ya que es invisible para ellos hasta que realmente la necesitan (al agregarla a la implementación).

Answer 3

Para mí, la palabra operativa es "umbral". Otra palabra para esto sería probablemente "olor".

Las cosas siempre están en un equilibrio. Parece (en este caso) que el centro del equilibrio está en cohesión (frío); y como solo tienes un pequeño número de duplicados, no es difícil de administrar.

Si tuvieras algún "evento" importante y pasaras a "1000" duplicados, entonces la balanza habría cambiado y entonces podrías acercarte.

Para mí, algunos duplicados manejables no son una señal para refactorizar (todavía); pero lo vigilaría.

Answer 4

Herencia es su amigo!

No duplicar el código. Incluso si una sola línea de código es muy larga o difícil, refactorícela a un método separado con un nombre distintivo. Piense en ello como alguien que leerá su código en un año. Si nombra esta función como "blabla", ¿sabrá el siguiente hombre qué hace esta función sin leer su código? Si no, debes cambiar el nombre. Después de una semana de pensar así te acostumbrarás y tu código será 12% ¡más legible! ;)

class SocketAction { 

    private static abstract class CreateSessionLoginHandler extends LoginHandler { 
     @Override 
     protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException { 
      Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID()); 
      socketAction.registerSession(); 
      socketAction._sess.runApplication(); 
     } 
    } 

    private static class AlwaysCreateSessionLoginHandler extends CreateSessionLoginHandler; 

    private static class AutoConnectAnyDeviceLoginHandler extends CreateSessionLoginHandler { 
     @Override 
     protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException { 
      if (Server.isUserRegistered(socketAction._sess.getUserLogin())) { 
       Log.logSysInfo("Session autoconnect - acquiring list of action threads..."); 
       String[] sa = Server.getSessionList(socketAction._sess.getUserID()); 
       Log.logSysInfo("Session autoconnect - list of action threads acquired."); 
       for (int i = 0; i < sa.length; i += 7) { 
        socketAction.abandonCommThreads(); 
        Server.attachSocketToSession(sa[i + 1], socketAction._commSendThread.getSock()); 
        return; 
       } 
      } 
      super.onLoginCorrect(socketAction); 
     } 
    } 

    private static class OnlyNewSessionLoginHandler extends CreateSessionLoginHandler { 
     @Override 
     protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException { 
      socketAction.killOldSessionsForUser(); 
      super.onLoginCorrect(socketAction); 
     } 
    } 
} 

Answer 5

Para mí, la prueba de fuego es: si tengo que hacer un cambio a esta secuencia de código en un mismo lugar, (por ejemplo, añadir una línea, cambiar el orden), qué tengo que hacer el mismo cambio en otras ubicaciones?

Si la respuesta es sí, entonces es una entidad "atómica" lógica y debe ser refactorizada. Si la respuesta es no, entonces se trata de una secuencia de operaciones que funcionan en más de una situación, y si se refabrican, es probable que le causen más problemas en el futuro.