Czy skomentowany kod może być cenną dokumentacją?

Napisałem następujący kod:

if (boutique == null) {
    boutique = new Boutique();

    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.persist(boutique);
} else {
    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    //boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.merge(boutique);
}

Jest tutaj komentarz z komentarzem. Ale myślę, że dzięki temu kod staje się bardziej przejrzysty, ponieważ jest oczywiste, jaka jest różnica między ifi else. Różnica jest jeszcze bardziej zauważalna dzięki podświetlaniu kolorów.

Czy komentowanie takiego kodu może być dobrym pomysłem?

Większość odpowiedzi skupia się na sposobie refaktoryzacji tego konkretnego przypadku, ale pozwólcie, że przedstawię ogólną odpowiedź na pytanie, dlaczego skomentowany kod jest zwykle zły:

Po pierwsze, skomentowany kod nie jest kompilowany. To oczywiste, ale oznacza to, że:

1. Kod może nawet nie działać.

2. Kiedy zmieniają się zależności komentarza, to oczywiście nie ulegnie awarii.

Skomentowany kod jest bardzo „martwym kodem”. Im dłużej tam siedzi, tym bardziej gnije i zapewnia coraz mniej wartości następnemu deweloperowi.

Po drugie, cel jest niejasny. Naprawdę potrzebujesz dłuższego komentarza, który zapewnia kontekst dla tego, dlaczego istnieją losowo komentowane wiersze. Kiedy widzę tylko skomentowany wiersz kodu, muszę zbadać, jak się tam dostał, aby zrozumieć, dlaczego się tam dostał. Kto to napisał? Co popełnia? Jaki był komunikat / kontekst zatwierdzenia? Etcetera.

Rozważ alternatywy:

• Jeśli celem są dostarczenie przykładów użycia funkcji / interfejsu API, to zapewnij test jednostkowy. Testy jednostkowe są prawdziwym kodem i zepsują się, gdy nie będą już poprawne.
• Jeśli celem jest zachowanie poprzedniej wersji kodu, użyj kontroli źródła. Wolałbym sprawdzić poprzednią wersję, a następnie przełączać komentarze w bazie kodu, aby „przywrócić” zmianę.
• Jeśli celem jest utrzymanie alternatywnej wersji tego samego kodu, użyj kontroli źródła (ponownie). W końcu po to są gałęzie.
• Jeśli celem jest wyjaśnienie struktury, zastanów się, jak możesz zrestrukturyzować kod, aby stał się bardziej oczywisty. Większość innych odpowiedzi to dobre przykłady tego, jak możesz to zrobić.

Największym problemem z tym kodem jest to, że zduplikowano te 6 wierszy. Po wyeliminowaniu tego powielania komentarz ten jest bezużyteczny.

Jeśli utworzysz boutiqueDao.mergeOrPersistmetodę, możesz przepisać to jako:

if (boutique == null) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

boutiqueDao.mergeOrPersist(boutique);

Kod, który tworzy lub aktualizuje określony obiekt, jest wspólny, dlatego należy go rozwiązać raz, na przykład tworząc mergeOrPersistmetodę. Z pewnością nie powielaj całego kodu przydziału dla tych dwóch przypadków.

Wiele ORM ma wbudowane wsparcie w tym zakresie. Na przykład mogą utworzyć nowy wiersz, jeśli idwynosi zero, i zaktualizować istniejący wiersz, jeśli idnie jest zero. Dokładna forma zależy od danego ORM, a ponieważ nie znam technologii, której używasz, nie mogę ci w tym pomóc.

Jeśli nie chcesz tworzyć mergeOrPersistmetody, powinieneś wyeliminować duplikację w inny sposób, na przykład poprzez wprowadzenie isNewBoutiqueflagi. To może nie być ładne, ale wciąż jest znacznie lepsze niż powielanie całej logiki przypisania.

bool isNewBoutique = boutique == null;
if (isNewBoutique) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

if (isNewBoutique)
    boutiqueDao.persist(boutique);
else
    boutiqueDao.merge(boutique);

To absolutnie przerażający pomysł. Nie wyjaśnia, co to za intencja. Czy programista przez pomyłkę skomentował linię? Testować coś? Co się dzieje?!

Poza tym widzę 6 linii, które są absolutnie równe w obu przypadkach. Zamiast tego powinieneś zapobiec temu duplikowaniu kodu. Wtedy będzie jasne, że w jednym przypadku dodatkowo wywołujesz setSelected.

Nie, to okropny pomysł. Na podstawie tego fragmentu kodu przychodzą mi na myśl następujące myśli:

• Ten wiersz został skomentowany, ponieważ programista go debugował i zapomniał przywrócić wiersz do poprzedniego stanu
• Ten wiersz został skomentowany, ponieważ kiedyś był częścią logiki biznesowej, ale już tak nie jest
• Ten wiersz został skomentowany, ponieważ spowodował problemy z wydajnością produkcji, a programista chciał zobaczyć, jaki wpływ miał on na system produkcyjny

Po zobaczeniu tysięcy wierszy skomentowanego kodu, teraz robię jedyną sensowną rzecz, kiedy go widzę: natychmiast go usuwam.

Nie ma rozsądnego powodu, aby zameldować skomentowany kod w repozytorium.

Ponadto twój kod wykorzystuje dużo powielania. Sugeruję jak najszybsze zoptymalizowanie tego dla czytelności dla ludzi.

Chciałbym tylko dodać do odpowiedzi CodesInChaos, wskazując, że można przełożyć ją na mniejsze metody . Udostępnianie wspólnej funkcjonalności według kompozycji pozwala uniknąć warunkowych:

function fill(boutique) {    
  boutique.setSite(site);
  boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
  boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
  boutique.setNom(fluxBoutique.getNom());
  boutique.setIdWebSC(fluxBoutique.getId());
  boutique.setDateModification(new Date());
}    

function create() {
  boutique = new Boutique();      
  fill(boutique);
  boutique.setSelected(false);
  return boutiqueDao.persist(boutique);
}

function update(boutique) {
  fill(boutiquie);
  return boutiquieDao.merge(boutique); 
}

function createOrUpdate(boutique) {
  if (boutique == null) {
    return create();
  }
  return update(boutique);  
}

Chociaż najwyraźniej nie jest to dobry przypadek skomentowania kodu, istnieje sytuacja, która moim zdaniem uzasadnia to:

// The following code is obvious but does not work because of <x>
// <offending code>
<uglier answer that actually does work>

Jest to ostrzeżenie dla każdego, kto zobaczy to później, że oczywista poprawa nie jest.

Edycja: Mówię o czymś małym. Jeśli jest duży, zamiast tego wyjaśnij.

W tym konkretnym przykładzie uważam, że skomentowany kod jest bardzo niejednoznaczny, głównie z powodów przedstawionych w odpowiedzi Dibkke . Inni sugerowali sposoby, w jakie można by zmienić kod, aby uniknąć nawet pokusy, aby to zrobić, jednak jeśli z jakiegoś powodu nie jest to możliwe (np. Linie są podobne, ale nie dość blisko), byłbym wdzięczny za komentarz:

// Nie musisz odznaczyć tego butiku, ponieważ [WHATEVER]

Myślę jednak, że istnieją sytuacje, w których pozostawienie (lub nawet dodanie komentarza) kodu nie jest naganne. Używając czegoś takiego jak MATLAB lub NumPY, często można napisać równoważny kod, który albo 1) iteruje po tablicy, przetwarzając jeden element na raz lub 2) obsługuje całą tablicę na raz. W niektórych przypadkach ten drugi jest znacznie szybszy, ale także znacznie trudniejszy do odczytania. Jeśli zastąpię jakiś kod jego wektoryzowanym odpowiednikiem, osadzę oryginalny kod w pobliskim komentarzu, na przykład:

%% Poniższy kod wektorowy zawiera następujące elementy:

% for ii in 1:N
%    for jj in 1:N
%      etc.

%, ale wersja matrycy działa około 15 razy szybciej na typowych danych wejściowych (MK, 03/10/2013)

Oczywiście należy zadbać o to, aby obie wersje rzeczywiście robiły to samo i aby komentarz albo był zsynchronizowany z rzeczywistym kodem, albo został usunięty, jeśli kod się zmieni. Oczywiście obowiązują również zwykłe zastrzeżenia dotyczące przedwczesnej optymalizacji ...

Jedyny raz, kiedy użyto skomentowanego kodu, był użyteczny w plikach konfiguracyjnych, w których kod dla każdej opcji jest podany, ale skomentowany, co ułatwia włączenie ustawień poprzez usunięcie znaczników komentarzy:

## Enable support for mouse input:
# enable_mouse = true

W takim przypadku skomentowany kod pomaga udokumentować wszystkie dostępne opcje i sposób ich użycia. Konwencjonalne jest również stosowanie wartości domyślnych, więc kod dokumentuje również ustawienia domyślne.

Ogólnie rzecz biorąc, kod jest samodokumentujący tylko dla osoby, która go napisała. Jeśli wymagana jest dokumentacja, napisz dokumentację. Niedopuszczalne jest oczekiwanie, że nowy programista w bazie kodu źródłowego usiądzie i przeczyta tysiące wierszy kodu, aby spróbować dowiedzieć się z wysokiego poziomu, co się dzieje.

W takim przypadku celem zakomentowanego wiersza kodu jest pokazanie różnicy między dwoma wystąpieniami duplikatu kodu. Zamiast próbować subtelnie dokumentować różnicę za pomocą komentarza, przepisz kod, aby miał on sens. Następnie, jeśli nadal uważasz, że konieczne jest skomentowanie kodu, napisz odpowiedni komentarz.

Nie, skomentowany kod staje się przestarzały i wkrótce staje się gorszy niż bezwartościowy, często jest szkodliwy, ponieważ cementuje pewien aspekt implementacji wraz ze wszystkimi obecnymi założeniami.

Komentarze powinny zawierać szczegóły interfejsu i zamierzoną funkcję; „zamierzona funkcja”: może obejmować, najpierw próbujemy tego, potem próbujemy tego, a następnie zawodzimy w ten sposób.

Programiści, których widziałem, starają się zostawiać rzeczy w komentarzach, są po prostu zakochani w tym, co napisali, nie chcą tego stracić, nawet jeśli nie dodaje niczego do gotowego produktu.

Może się zdarzyć w bardzo rzadkich przypadkach, ale nie tak, jak to zrobiłeś. Inne odpowiedzi całkiem dobrze wyjaśniły powody tego.

Jednym z rzadkich przypadków jest szablon RPM, którego używamy w moim sklepie jako punkt wyjścia dla wszystkich nowych pakietów, głównie po to, aby upewnić się, że nic ważnego nie zostanie pominięte. Większość, ale nie wszystkie nasze pakiety mają archiwum zawierające źródła, które mają standardową nazwę i są oznaczone tagiem:

Name:           foomatic
Version:        3.14
 ...
Source0:        %{name}-%{version}.tar.gz

W przypadku pakietów bez źródeł komentujemy tag i umieszczamy nad nim kolejny komentarz, aby zachować standardowy format i wskazać, że ktoś zatrzymał się i pomyślał o problemie w ramach procesu programowania:

Name:           barmatic
Version:        2.71
 ...
# This package has no sources.
# Source0:        %{name}-%{version}.tar.gz

Nie dodajesz kodu, o którym wiesz, że nie będzie używany, ponieważ, jak inni to opisali, można go pomylić z czymś, co do niego należy. To może. warto jednak dodać komentarz wyjaśniający, dlaczego brakuje kodu, którego można się spodziewać:

if ( condition ) {
  foo();
  // Under most other circumstances, we would do a bar() here, but
  // we can't because the quux isn't activated yet.  We might call
  // bletch() later to rectify the situation.
  baz();
}

Skomentowany kod nie jest używany przez aplikację, dlatego należy mu towarzyszyć dalsze komentarze wyjaśniające , dlaczego nie jest on używany. Ale w tym kontekście, że są sytuacje, w których kod zakomentowanych mogą być użyteczne.

Moim zdaniem przychodzi przypadek, w którym rozwiązujesz problem przy użyciu wspólnego i atrakcyjnego podejścia, ale okazuje się, że wymagania dotyczące rzeczywistego problemu nieco się różnią od tego problemu. Zwłaszcza jeśli twoje wymagania wymagają znacznie więcej kodu, pokusa opiekunów do „optymalizacji” kodu przy użyciu starego podejścia prawdopodobnie będzie silna, ale zrobienie tego tylko przywróci błąd. Utrzymanie „złej” implementacji w komentarzach pomoże to rozwiązać, ponieważ możesz użyć jej do zilustrowania, dlaczego takie podejście jest złe w tej sytuacji.

Nie wyobrażam sobie takiej sytuacji, która zdarza się bardzo często. Zwykle powinno wystarczyć wyjaśnienie rzeczy bez uwzględnienia przykładowej „złej” implementacji. Ale można sobie wyobrazić sytuację, w której to nie wystarczy, więc skoro jest pytanie o to, czy może to być przydatne, tak, że można. Po prostu nie przez większość czasu.

To nie wygląda dobrze, stary.

Skomentowany kod to ... po prostu nie kod. Kod służy do implementacji logiki. Uczynienie kodu bardziej czytelnym jest sztuką. Jak @CodesInChaos sugerują już, że powtarzające się wiersze kodu nie są bardzo dobrą implementacją logiki .

Czy naprawdę uważasz, że jeden prawdziwy programista woli czytelność niż logiczną implementację. (nawiasem mówiąc, mamy komentarze i „uzupełnienia” w celu przedstawienia naszej logicznej reprezentacji).

Moim zdaniem należy napisać kod dla kompilatora i to dobrze - jeśli „rozumie” ten kod. Dla czytelności ludzi Komentarze są dobre, dla programistów (na dłuższą metę), dla osób ponownie używających tego kodu (np. Testerów).

W przeciwnym razie możesz wypróbować tutaj coś bardziej elastycznego, na przykład

boutique.setSite (strona) można zastąpić

setsiteof.boutique (strona). Istnieją różne aspekty i perspektywa OOP, dzięki którym można zwiększyć czytelność.

Chociaż ten kod wydaje się początkowo bardzo atrakcyjny i można pomyśleć, że znalazł sposób na czytelność dla człowieka, podczas gdy kompilator wykonuje również swoje zadanie doskonale, a my wszyscy zaczynamy postępować zgodnie z tą praktyką, doprowadzi to do powstania rozmytego pliku, który stanie się mniej czytelny z czasem i bardziej skomplikowane, ponieważ będzie się rozszerzać.