Aby suszyć lub nie suszyć? O unikaniu duplikacji kodu i zachowaniu spójności

Question

Mam pytanie dotyczące powielania kodu i refaktoryzacji, mam nadzieję, że nie jest zbyt ogólny. Załóżmy, że masz dość mały fragment kodu (~ 5 linii), który jest sekwencją wywołań funkcji , która jest - nie jest bardzo niskim poziomem. Ten kod powtarza się w kilku miejscach, więc dobrym pomysłem byłoby wyodrębnienie metody tutaj. Jednak w tym konkretnym przykładzie ta nowa funkcja ucierpiałaby z powodu niskiej spójności (co przejawia się, między innymi, trudnością w znalezieniu dobrego imienia dla funkcji). Powodem tego jest prawdopodobnie to, że ten powtarzający się kod jest tylko częścią większego algorytmu - i trudno jest go podzielić na dobrze nazwane kroki.

Co proponujesz w takim scenariuszu?

Edit:

Chciałem zachować pytanie na poziomie ogólnym, tak aby więcej osób mogło potencjalnie znaleźć przydatne, ale oczywiście najlepiej byłoby jego kopię zapasową z jakiejś próbki kodu. Przykładem może być jednym z najlepszych w historii (pachnie w dość kilka sposobów), ale mam nadzieję, że spełni swoje zadanie:

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

pytanie jest zbyt ogólne, aby naprawdę powiedzieć, ale jako ćwiczenie:

Załóżmy, że je zredukujesz. Zastanów się, jakie są prawdopodobne powody, dla których chcesz zmienić wynikową funkcję 5-liniową. Czy chcesz najprawdopodobniej wprowadzić zmiany dotyczące wszystkich użytkowników, czy też musiałbyś napisać nową funkcję, która różni się od starej, za każdym razem, gdy jakiś rozmówca ma powody, by chcieć zmiany?

Jeżeli chcesz zmienić go dla wszystkich użytkowników, jest to opłacalne abstrakcją. Daj mu teraz słabe imię, możesz później wymyślić coś lepszego.

Jeśli masz zamiar skończyć dzielenie tę funkcję w wielu podobnych wersjach jako kod ewoluuje w przyszłości, to chyba nie realną abstrakcją. Nadal można napisać funkcję, ale jest to raczej "oszczędzająca" funkcja "pomocnika" kodu, niż część formalnego modelu problemu. To nie jest zbyt satysfakcjonujące: powtórzenie tej ilości kodu jest nieco niepokojące, ponieważ sugeruje, że powinno być tam, które może być realną abstrakcją.

Może 4 z 5 linii mogą być wydobywane na zewnątrz, ponieważ są one bardziej spójna, a piąta linia właśnie tak dzieje się kręci z nimi w tej chwili. Wtedy możesz napisać 2 nowe funkcje: jedną, która jest tą nową abstrakcją, a druga jest tylko pomocnikiem, który wywołuje nową funkcję, a następnie wykonuje wiersz 5. Jedna z tych funkcji może mieć dłuższy oczekiwany okres użytkowania niż inny. .

Answer 2

Myślałam o tym ostatnio i rozumiem dokładnie co ci chodzi. Wydaje mi się, że jest to abstrakcja implementacji bardziej niż cokolwiek innego, a zatem jest bardziej smaczna, jeśli można uniknąć zmiany interfejsu. Na przykład w C++ mogę wyodrębnić funkcję do cpp bez dotykania nagłówka. To nieco zmniejsza wymaganie, aby abstrakcja funkcji była dobrze sformułowana i znacząca dla użytkownika klasy, ponieważ jest dla nich niewidoczna, dopóki naprawdę jej nie potrzebuje (podczas dodawania do implementacji).

Answer 3

Dla mnie operatywne słowo to "próg". Innym słowem na to prawdopodobnie byłby "zapach".

Rzeczy są zawsze w równowadze. Brzmi jak (w tym przypadku), jak środek równowagi jest w spoistości (cool); a ponieważ masz tylko niewielką liczbę duplikatów, nie jest trudno zarządzać.

Jeśli pojawiło się jakieś większe "wydarzenie", a użytkownik przeszedł do duplikatu "1000", wówczas saldo ulegnie zmianie, a Ty możesz podejść.

Dla mnie kilka managable duplikatów nie jest sygnałem do refaktoryzacji (jeszcze); ale powinienem mieć na to oko.

Answer 4

Dziedziczenie to Twój przyjaciel!

Nie duplikuj kodu. Nawet jeśli pojedyncza linia kodu jest bardzo długa lub trudna, należy ją przekształcić w osobną metodę o wyróżniającej nazwie. Pomyśl o tym jak o kimś, kto przeczyta Twój kod w ciągu roku. Jeśli nazwiesz tę funkcję "blabla", to czy następny facet będzie wiedział, co robi ta funkcja, nie odczytując jej kodu? Jeśli nie, musisz zmienić nazwę. Po tygodniu myślenia w ten sposób przyzwyczaisz się do tego, a Twój kod będzie bardziej czytelny niż Twój 12%! ;)

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

Dla mnie papierkiem lakmusowym jest: jeśli trzeba wprowadzić zmiany w tej sekwencji kodu w jednym miejscu (np dodać linię, zmienić kolejność), musiałbym zrobić ta sama zmiana w innych lokalizacjach?

Jeśli odpowiedź brzmi tak, to jest to logiczna, "atomowa" istota i powinna być refaktoryzowana. Jeśli odpowiedź brzmi "nie", to jest to sekwencja operacji, które działają w więcej niż jednej sytuacji - a jeśli zostaną refaktoryzowane, prawdopodobnie spowodują więcej problemów w przyszłości.