Czy dopuszczalne jest kopiowanie i wklejanie długiego, ale prostego kodu zamiast owijania go w klasę lub funkcję?


29

Załóżmy, że mam segment kodu do połączenia z Internetem i pokazania wyników takiego połączenia:

HttpRequest* httpRequest=new HttpRequest();
httpRequest->setUrl("(some domain .com)");
httpRequest->setRequestType(HttpRequest::Type::POST);
httpRequest->setRequestData("(something like name=?&age=30&...)");
httpRequest->setResponseCallback([=](HttpClient* client, HttpResponse* response){
    string responseString=response->getResponseDataString();
        if(response->getErrorCode()!=200){
            if(response->getErrorCode()==404){
                Alert* alert=new Alert();
                alert->setFontSize(30);
                alert->setFontColor(255,255,255);
                alert->setPosition(Screen.MIDDLE);
                alert->show("Connection Error","Not Found");
            }else if((some other different cases)){
                (some other alert)
            }else
                Alert* alert=new Alert();
                alert->setFontSize(30);
                alert->setPosition(Screen.MIDDLE);
                alert->setFontColor(255,255,255);
                alert->show("Connection Error","unknown error");
            }
        }else{
            (other handle methods depend on different URL)
        }
}

kod jest długi i jest powszechnie używany, ale powyższy kod nie wymaga żadnych dodatkowych rzeczy, takich jak niestandardowa funkcja i klasa (HttpRequest i Alert są domyślnie dostarczane przez framework), i chociaż segment kodu jest długi, to jest proste i nieskomplikowane (jest długie tylko dlatego, że istnieją pakiety ustawień, takie jak adres URL, rozmiar czcionki ...), a segment kodu ma niewielkie różnice między klasami (np .: adres URL, dane żądania, przypadki obsługi kodu błędu, normalny uchwyt skrzynie ...)

Moje pytanie brzmi: czy dopuszczalne jest kopiowanie i wklejanie długiego, ale prostego kodu zamiast zawijania ich w funkcji w celu zmniejszenia zależności kodu?


89
Wyobraź sobie, że masz błąd w tym kodzie, taki jak brak zwalniania przydzielanych obiektów. (Czy twój framework uwalnia Alertobiekty?) Teraz wyobraź sobie, że musisz znaleźć każdą skopiowaną instancję tego kodu, aby naprawić błąd. Teraz wyobraź sobie, że to nie ty musisz to zrobić, ale szalony morderca siekierą, który wie, że to ty stworzyłeś wszystkie te kopie.
Sebastian Redl,

8
A BTW, połączenie sieci i wyświetlania błędów w jednym miejscu jest już dużym nie-nie, IMHO.
śleske,

11
Nie, nigdy. Całkowicie niedopuszczalne. Gdybyś był przy moim projekcie, nie byłbyś już przy moim projekcie i byłbyś na programie szkoleniowym lub PIP.
nhgrif

10
I pojawia się przełożony, który mówi: „To ostrzeżenie na środku mojego ekranu jest absolutnym przerażeniem. Oglądam wyskakujące koty, a wyskakujące okienko blokuje mój widok za każdym razem, gdy się pojawia. Proszę przenieść go w prawy górny róg . ” 3 tygodnie później „Co ty do cholery zrobiłeś ?! Nie mogę już zamykać kotów, ponieważ TWÓJ wyskakujący okrywa X w prawym górnym rogu, napraw to”.
MonkeyZeus

11
Myślę, że wszyscy tutaj myślą, że to zły pomysł. Ale żeby odwrócić pytanie, dlaczego NIE umieściłbyś tego kodu w osobnej klasie lub funkcji?
Karl Gjertsen

Odpowiedzi:


87

Musisz wziąć pod uwagę koszt zmiany. Co jeśli chcesz zmienić sposób nawiązywania połączeń? Jak łatwo by to było? Jeśli masz dużo zduplikowanego kodu, znalezienie wszystkich miejsc, które wymagają zmiany, może być dość czasochłonne i podatne na błędy.

Musisz także wziąć pod uwagę jasność. Najprawdopodobniej przeglądanie 30 wierszy kodu nie będzie tak łatwe do zrozumienia, jak pojedyncze wywołanie funkcji „connectToInternet”. Ile czasu trzeba stracić, próbując zrozumieć kod, gdy trzeba dodać nową funkcjonalność?

W niektórych rzadkich przypadkach duplikacja nie stanowi problemu. Na przykład, jeśli przeprowadzasz eksperyment, a kod zostanie wyrzucony pod koniec dnia. Ale ogólnie rzecz biorąc, koszt duplikacji przewyższa niewielkie oszczędności czasu związane z brakiem konieczności wyciągania kodu do osobnej funkcji .

Zobacz także https://softwareengineering.stackexchange.com/a/103235/63172


19
... i kto wie, czy te 30 linii jest naprawdę takich samych, jak te, które oglądałeś wcześniej, czy też ktoś zmienił inny port lub adres IP w swojej kopii z jakiegokolwiek powodu. Domniemane założenie, że „ jakakolwiek wiązka około 30 wierszy zaczynających się od HttpRequesttego samego ” jest łatwa do popełniania błędu.
null

@ null Excellent point. Kiedyś pracowałem nad kodem, w którym skopiowano i wklejono połączenia z bazą danych, ale niektóre miały subtelne różnice w konfiguracji. Nie miałem pojęcia, czy były to ważne, celowe zmiany, czy tylko przypadkowe różnice
user949300,

54

Nie.

W rzeczywistości nawet Twój „prosty” kod powinien być podzielony na mniejsze części. Co najmniej dwa

Jeden, aby nawiązać połączenie i obsłużyć normalną odpowiedź 200. Na przykład co się stanie, jeśli w niektórych przypadkach zmienisz POST na PUT? Co zrobić, jeśli tworzysz zilliony tych połączeń i potrzebujesz wielowątkowości lub puli połączeń? Posiadanie kodu w jednym miejscu, wraz z argumentem za metodą, znacznie to ułatwi

Podobnie inny do obsługi błędów. Na przykład jeśli zmienisz kolor lub rozmiar czcionki alertu. Lub masz problemy z przerywanymi połączeniami i chcesz zarejestrować błędy.



Możesz również zacytować SRP: blok kodu, który ma tylko jeden cel, znacznie ułatwia zrozumienie i utrzymanie ..
Roland Tepp

Jest to jeden przypadek, w którym DRY i SRP faktycznie się wyrównują. Czasem nie.
user949300

18

czy można kopiować i wklejać ...

Nie.

Dla mnie decydujący jest następujący argument:

... jest powszechnie używany ...

Jeśli użyjesz fragmentu kodu w więcej niż jednym miejscu, wtedy, gdy się zmieni, musisz go zmienić w więcej niż jednym miejscu lub zaczniesz mieć niespójności - zaczynają się dziać „dziwne rzeczy” (tj. Wprowadzasz błędy).

jest prosty i nieskomplikowany ...

I tak powinno być łatwiej refaktoryzować się do funkcji.

... istnieją pakiety ustawień, takie jak adres URL, rozmiar czcionki ...

A co użytkownicy lubią zmieniać? Czcionki, rozmiary czcionek, kolory itp. Itp.

Teraz; w ilu miejscach będziesz musiał zmienić ten sam fragment kodu, aby ponownie uzyskać ten sam kolor / czcionkę / rozmiar? (Zalecana odpowiedź: tylko jedna ).

... segment kodu ma niewielkie różnice między klasami (np. adres URL, dane żądania, przypadki obsługi kodów błędów, normalne przypadki obsługi ...)

Wariacja => parametry funkcji.


„A co użytkownicy uwielbiają zmieniać? Czcionki, rozmiary czcionek, kolory itp.” To jest bardzo ważne. Czy naprawdę chcesz to zmienić w kilkudziesięciu lokalizacjach?
Dan

8

To tak naprawdę nie ma nic wspólnego z kopiowaniem i wklejaniem. Jeśli weźmiesz kod z innego miejsca, w momencie, gdy go weźmiesz, będzie to twój kod i twoja odpowiedzialność, więc to, czy jest on kopiowany czy zapisywany całkowicie sam, nie ma znaczenia.

W swoich alertach podejmujesz pewne decyzje dotyczące projektu. Najprawdopodobniej dla wszystkich alertów należy podjąć podobne decyzje projektowe. Jest więc prawdopodobne, że powinieneś mieć gdzieś metodę „ShowAlertInAStyleSuitableForMyApplication” lub może być nieco krótszą i należy ją wywołać.

Będziesz mieć wiele żądań HTTP z podobną obsługą błędów. Prawdopodobnie nie powinieneś powtarzać obsługi błędów raz po raz, ale wyodrębnij typową obsługę błędów. Zwłaszcza jeśli obsługa błędów staje się nieco bardziej skomplikowana (co z błędami przekroczenia limitu czasu, 401 itd.).


6

Powielanie jest w niektórych przypadkach OK. Ale nie w tym. Ta metoda jest zbyt skomplikowana. Istnieje dolny limit, gdy duplikacja jest łatwiejsza niż „wyodrębnienie” metody.

Na przykład:

def add(a, b)
    return a + b
end

jest głupie, po prostu zrób + b.

Ale kiedy stajesz się trochę, trochę bardziej skomplikowany, wtedy zwykle przekraczasz granice.

foo.a + foo.b

powinno stać się

foo.total
def foo
    ...
    def total
        return self.a + self.b
    end
end

W twoim przypadku widzę cztery „metody”. Prawdopodobnie w różnych klasach. Jeden, aby wysłać żądanie, jeden, aby uzyskać odpowiedź, jeden, aby wyświetlić błędy, i pewnego rodzaju oddzwonienie, które zostanie wywołane po powrocie odpowiedzi w celu przetworzenia odpowiedzi. Osobiście prawdopodobnie dodałbym coś w rodzaju „opakowania”, aby ułatwić połączenia.

Na koniec, aby wysłać żądanie internetowe, chciałbym, aby połączenie wyglądało mniej więcej tak:

Web.Post(URI, Params, ResponseHandler);

Ta linia jest tym, co miałbym w całym kodzie. Potem, kiedy musiałem wprowadzić zmiany w „jak zdobyć rzeczy”, mogłem to zrobić szybko, przy znacznie mniejszym wysiłku.

Utrzymuje to również kod DRY i pomaga w SRP .


0

W projekcie dowolnej wielkości / złożoności chcę móc znaleźć kod, gdy jest potrzebny w następujących celach:

  1. Napraw to, gdy się zepsuje
  2. Zmień funkcjonalność
  3. Użyj tego ponownie.

Czy nie byłoby wspaniale dołączyć do projektu w toku lub kontynuować pracę nad projektem przez kilka lat, a kiedy nowa prośba o „połączenie z Internetem i pokazanie wyników połączenia” była w jakiejś łatwej do znalezienia lokalizacji z powodu posiadania dobry projekt zamiast polegać na przeszukiwaniu całego kodu dla httprequest? Tak czy inaczej, łatwiej jest go znaleźć w Google.

Nie martw się, jestem nowym facetem i przerobię ten blok kodu, ponieważ jestem bardzo zdenerwowany dołączeniem do tego nieświadomego zespołu z okropną bazą kodów lub jestem teraz pod dużą presją, jak reszta ty i po prostu skopiujesz i wkleisz go. Przynajmniej powstrzyma szefa od moich pleców. Następnie, gdy projekt zostanie naprawdę zidentyfikowany jako katastrofa, jako pierwszy zalecę przepisanie go przez skopiowanie i wklejenie wersji w najnowszym i najlepszym środowisku, którego nikt z nas nie rozumie.


0

Klasa i / lub funkcja jest lepsza, przynajmniej moim zdaniem. Po raz pierwszy zmniejsza rozmiar pliku, co jest bardzo dużym zyskiem, jeśli masz do czynienia z aplikacjami internetowymi lub aplikacjami dla urządzeń z małą ilością miejsca (Internet Rzeczy, starsze telefony itp.)

I oczywiście najlepsza rzecz jest taka, że ​​jeśli masz coś do zmiany ze względu na nowe protokoły itp., Po prostu zmieniasz zawartość funkcji i niezliczoną liczbę razy umieszczasz tę funkcję gdzieś, która może znajdować się nawet w różnych plikach, co czyni je jeszcze trudniejszymi znajdź i zmień.

Napisałem cały interpreter SQL, więc mogłem lepiej przestawić się z MySQL na MySQLi w PHP, ponieważ muszę tylko zmienić interpreter i wszystko działa, chociaż to trochę ekstremalny przykład.


-1

Aby zdecydować, czy kod powinien zostać zduplikowany, czy przeniesiony do funkcji wywoływanej dwukrotnie, spróbuj ustalić, która jest bardziej prawdopodobna:

  1. Konieczna będzie zmiana obu sposobów użycia kodu w ten sam sposób.

  2. Konieczna będzie zmiana co najmniej jednego użycia kodu, aby się różniły.

W pierwszym przypadku prawdopodobnie lepiej będzie mieć jedną funkcję obsługującą oba zastosowania; w tym drugim przypadku prawdopodobnie lepiej będzie mieć osobny kod dla dwóch zastosowań.

Decydując, czy fragment kodu, który zostanie użyty raz, powinien zostać napisany w wierszu, czy wyciągnięty do innej funkcji, dowiedz się, jak można w pełni opisać wymagane zachowanie funkcji. Jeśli pełny i dokładny opis wymaganego zachowania funkcji byłby tak długi lub dłuższy niż sam kod, przeniesienie kodu do oddzielnej funkcji może sprawić, że będzie trudniej zrozumieć niż łatwiej. Warto nadal robić, jeśli istnieje duże prawdopodobieństwo, że drugi dzwoniący będzie musiał użyć tej samej funkcji, a wszelkie przyszłe zmiany funkcji będą musiały wpłynąć na obu dzwoniących, ale bez takiego uwzględnienia czytelność sprzyja podziałowi do poziomu, na którym kod i opis jego wymaganego zachowania byłby mniej więcej taki sam.

Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.