Kiedy paradygmat „Do One Thing” staje się szkodliwy?

Dla celów argumentu oto przykładowa funkcja, która drukuje zawartość danego pliku linia po linii.

Wersja 1:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  string line;
  while (std::getline(file, line)) {
    cout << line << endl;
  }
}

Wiem, że zalecane jest, aby funkcje wykonywały jedną rzecz na jednym poziomie abstrakcji. Dla mnie powyższy kod robi prawie jedną rzecz i jest dość atomowy.

Niektóre książki (takie jak Czysty kod Roberta C. Martina) wydają się sugerować podzielenie powyższego kodu na osobne funkcje.

Wersja 2:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  printLines(file);
}

void printLines(fstream & file) {
  string line;
  while (std::getline(file, line)) {
    printLine(line);
  }
}

void printLine(const string & line) {
  cout << line << endl;
}

Rozumiem, co chcą osiągnąć (otwórz plik / przeczytaj linie / wydrukuj linię), ale czy nie jest to trochę przesada?

Oryginalna wersja jest prosta i w pewnym sensie już robi jedną rzecz - drukuje plik.

Druga wersja doprowadzi do dużej liczby naprawdę małych funkcji, które mogą być znacznie mniej czytelne niż pierwsza wersja.

Czy nie byłoby w tym przypadku lepiej mieć kod w jednym miejscu?

W którym momencie paradygmat „Do One Thing” staje się szkodliwy?

Oczywiście, to tylko nasuwa pytanie „Co to jest jedna rzecz?” Czy czytanie linii to jedno, a pisanie innego? Czy też kopiowanie wiersza z jednego strumienia do drugiego można uznać za jedną rzecz? Lub kopiowanie pliku?

Nie ma na to twardej, obiektywnej odpowiedzi. To zależy od Ciebie. Możesz zdecydować. Ty musisz zdecydować. Głównym celem paradygmatu „rób jedną rzecz” jest prawdopodobnie stworzenie kodu tak łatwego do zrozumienia, jak to tylko możliwe, abyś mógł użyć go jako wskazówki. Niestety, nie jest to również obiektywnie mierzalne, więc musisz polegać na swoich przeczuciach i „WTF?” liczyć w przeglądzie kodu .

IMO funkcja składająca się tylko z jednego wiersza kodu rzadko jest warta kłopotów. Nie printLine()ma przewagi nad bezpośrednim użyciem std::cout << line << '\n'¹ . Jeśli widzę printLine(), muszę albo założyć, że robi to, co mówi jego nazwa, albo sprawdzić i sprawdzić. Jeśli widzę std::cout << line << '\n', od razu wiem, co robi, ponieważ jest to kanoniczny sposób wyprowadzania zawartości ciągu jako linii do std::cout.

Jednak kolejnym ważnym celem tego paradygmatu jest umożliwienie ponownego użycia kodu, a to jest o wiele bardziej obiektywny środek. Na przykład w drugiej wersji printLines() można łatwo napisać, aby był to uniwersalnie przydatny algorytm kopiujący linie z jednego strumienia do drugiego:

void copyLines(std::istream& is, std::ostream& os)
{
  std::string line;
  while( std::getline(is, line) );
    os << line << '\n';
  }
}

Taki algorytm mógłby zostać ponownie wykorzystany również w innych kontekstach.

Następnie możesz umieścić wszystko specyficzne dla tego jednego przypadku użycia w funkcji, która wywołuje ten ogólny algorytm:

void printFile(const std::string& filePath) {
  std::ifstream file(filePath.c_str());
  printLines(file, std::cout);
}

^{1 _{Zauważ, że użyłem '\n'raczej niż std::endl. '\n'powinien być domyślnym wyborem dla wypisywania nowego wiersza , std::endlto nieparzysty przypadek .}}

Wykonywanie funkcji tylko „jedna rzecz” oznacza środek do dwóch pożądanych celów, a nie przykazanie od Boga:

1. Jeśli twoja funkcja wykonuje tylko „jedną rzecz”, pomoże ci to uniknąć duplikacji kodu i rozdęcia API, ponieważ będziesz w stanie komponować funkcje w celu wykonania bardziej złożonych rzeczy zamiast kombinatorycznej eksplozji funkcji wyższego poziomu, mniej dających się skomponować .

2. Posiadanie funkcji wykonujących tylko jedną rzecz może sprawić, że kod będzie bardziej czytelny. Zależy to od tego, czy zyskujesz większą jasność i łatwość rozumowania, oddzielając rzeczy, niż tracisz na szczegółowości, pośredniczości i narzutach konstrukcyjnych, które pozwalają ci oddzielić rzeczy.

Dlatego „jedna rzecz” jest nieuchronnie subiektywna i zależy od tego, jaki poziom abstrakcji jest istotny dla twojego programu. Jeśli printLinesjest uważany za pojedynczą, podstawową operację i jedyny sposób drukowania linii, na którym ci zależy lub na którą masz nadzieję, to printLinestylko dla jednej rzeczy. Chyba że druga wersja jest bardziej czytelna (ja nie), pierwsza wersja jest w porządku.

Jeśli zaczniesz potrzebować większej kontroli nad niższymi poziomami abstrakcji, a skończysz na subtelnym powielaniu i eksplozji kombinatorycznej (tj. printLinesDla nazw plików i całkowicie oddzielny printLinesdla fstreamobiektów, printLinesdla konsoli i printLinesdla plików), wtedy printLinesrobi więcej niż jedną rzecz na poziomie abstrakcji, na której ci zależy.

W tej skali to nie ma znaczenia. Implementacja jednofunkcyjna jest całkowicie oczywista i zrozumiała. Jednak dodanie nieco większej złożoności sprawia, że ​​bardzo atrakcyjne jest podzielenie iteracji od akcji. Załóżmy na przykład, że musisz wydrukować linie z zestawu plików określonego przez wzorzec taki jak „* .txt”. Następnie oddzieliłbym iterację od akcji:

printLines(FileSet files) {
   files.each({ 
       file -> file.eachLine({ 
           line -> printLine(line); 
       })
   })
}

Teraz iterację plików można osobno przetestować.

Dzielę funkcje, aby uprościć testowanie lub poprawić czytelność. Gdyby akcja wykonana na każdej linii danych była wystarczająco złożona, aby uzasadnić komentarz, z pewnością podzieliłbym go na osobną funkcję.

Wyodrębnij metody, gdy poczujesz potrzebę komentarza w celu wyjaśnienia rzeczy.

Napisz metody, które albo robią to, co mówi ich nazwa w oczywisty sposób, albo opowiedz historię, wywołując sprytnie nazwane metody.

Nawet w twoim prostym przypadku brakuje Ci szczegółów, że zasada jednolitej odpowiedzialności pomogłaby ci lepiej zarządzać. Na przykład, co się dzieje, gdy coś pójdzie nie tak z otwarciem pliku. Dodanie obsługi wyjątków w celu zabezpieczenia przed przypadkami krawędzi dostępu do plików dodałoby 7-10 linii kodu do twojej funkcji.

Po otwarciu pliku nadal nie jesteś bezpieczny. Może być oderwany od ciebie (szczególnie jeśli jest to plik w sieci), możesz zabraknąć pamięci, ponownie może zdarzyć się kilka przypadków krawędzi, które chcesz zahartować i zepsuć swoją monolityczną funkcję.

Jednowarstwowa linia wydruku wydaje się wystarczająco niewinna. Ale wraz z dodawaniem nowych funkcji do drukarki plików (parsowanie i formatowanie tekstu, renderowanie na różnego rodzaju wyświetlaczach itp.) Będzie się powiększać, a później podziękujesz sobie.

Celem SRP jest umożliwienie ci myślenia o pojedynczym zadaniu na raz. To jak podzielenie dużego bloku tekstu na wiele akapitów, aby czytelnik mógł zrozumieć punkt, który próbujesz osiągnąć. Trochę więcej czasu zajmuje napisanie kodu zgodnego z tymi zasadami. Robiąc to, ułatwiamy czytanie tego kodu. Pomyśl, jak szczęśliwy będzie twój przyszły ja, gdy będzie musiał wyśledzić błąd w kodzie i znaleźć go porządnie podzielony na partycje.

Ja osobiście wolę to drugie podejście, ponieważ oszczędza ci to pracy w przyszłości i wymusza sposób myślenia „jak to zrobić w sposób ogólny”. Mimo to w twoim przypadku wersja 1 jest lepsza niż wersja 2 - tylko dlatego, że problemy rozwiązane przez wersję 2 są zbyt trywialne i specyficzne dla fstream. Myślę, że należy to zrobić w następujący sposób (w tym poprawkę błędu zaproponowaną przez Nawaza):

Ogólne funkcje narzędziowe:

void printLine(ostream& output, const string & line) { 
    output << line << endl; 
} 

void printLines(istream& input, ostream& output) { 
    string line; 
    while (getline(input, line)) {
        printLine(output, line); 
    } 
} 

Funkcja specyficzna dla domeny:

void printFile(const string & filePath, ostream& output = std::cout) { 
    fstream file(filePath, ios::in); 
    printLines(file, output); 
} 

Teraz printLinesi printLinemoże współpracować nie tylko fstreamz dowolnym strumieniem.

Każdy paradygmat (niekoniecznie ten, który zacytowałeś), którego należy przestrzegać, wymaga pewnej dyscypliny, a zatem ograniczenia „wolności słowa” - powoduje początkowy narzut (przynajmniej dlatego, że musisz się go nauczyć!). W tym sensie każdy paradygmat może stać się szkodliwy, gdy koszt tego narzutu nie zostanie nadmiernie zrekompensowany przez zaletę, jaką ten paradygmat zaprojektowano tak, aby był sam w sobie.

Prawdziwa odpowiedź na to pytanie wymaga zatem dobrej zdolności do „przewidywania” przyszłości, na przykład:

• Jestem teraz zobowiązany do zrobienia AiB
• Jakie jest prawdopodobieństwo, że w najbliższej przyszłości będę musiał to zrobić A-i B+(tj. Coś, co wygląda jak A i B, ale tylko trochę inaczej)?
• Jakie jest prawdopodobieństwo, że w dalszej przyszłości A + stanie się A*lub A*-?

Jeśli to prawdopodobieństwo jest względnie wysokie, to będzie duża szansa, jeśli - myśląc jednocześnie o A i B - zastanowię się również nad ich możliwymi wariantami, aby w ten sposób wyodrębnić części wspólne, aby móc je ponownie wykorzystać.

Jeśli to prawdopodobieństwo jest bardzo niskie (jakikolwiek wariant wokół Ajest zasadniczo niczym więcej niż Asobą samym), przestudiuj, jak rozłożyć A, najprawdopodobniej spowoduje zmarnowany czas.

Jako przykład pozwól, że opowiem ci tę prawdziwą historię:

Podczas mojego poprzedniego życia jako nauczyciel, odkryłem, że -on większość projects- studenta praktycznie wszystkie z nich zapewniają własną funkcję, aby obliczyć długość łańcucha C .

Po pewnym dochodzeniu odkryłem, że będąc częstym problemem, wszyscy uczniowie wpadli na pomysł, aby użyć do tego funkcji. Po powiedzeniu im, że istnieje funkcja biblioteczna ( strlen), wielu z nich odpowiedziało, że ponieważ problem był tak prosty i trywialny, bardziej efektywne było napisanie własnej funkcji (2 linii kodu) niż szukanie podręcznika biblioteki C (to był 1984 rok, zapomniałem WEB i google!) w ścisłej kolejności alfabetycznej, aby sprawdzić, czy jest na to gotowa funkcja.

Jest to przykład, w którym również paradygmat „nie wymyślaj na nowo koła” może stać się szkodliwy bez skutecznego katalogu kół!

Twój przykład jest odpowiedni do użycia w narzędziu do wyrzucania, które jest potrzebne wczoraj do wykonania określonego zadania. Lub jako narzędzie administracyjne, które jest bezpośrednio kontrolowane przez administratora. Teraz spraw, aby był solidny i odpowiedni dla Twoich klientów.

Dodaj odpowiednią obsługę błędów / wyjątków za pomocą znaczących komunikatów. Być może potrzebujesz weryfikacji parametrów, w tym decyzji, które należy podjąć, np. Jak obsługiwać nieistniejące pliki. Dodaj funkcję rejestrowania, być może z różnymi poziomami, takimi jak informacje i debugowanie. Dodaj komentarze, aby Twoi koledzy z zespołu wiedzieli, co się tam dzieje. Dodaj wszystkie części, które zwykle są pomijane ze względu na zwięzłość i pozostawione jako ćwiczenie dla czytelnika, podając przykłady kodu. Nie zapomnij o testach jednostkowych.

Twoja ładna i dość liniowa mała funkcja nagle kończy się złożonym bałaganem, który zaczyna być dzielony na osobne funkcje.

IMO staje się szkodliwe, gdy posuwa się tak daleko, że funkcja prawie nic nie robi, ale deleguje pracę do innej funkcji, ponieważ jest to znak, że nie jest to już abstrakcja niczego, a sposób myślenia prowadzący do takich funkcji zawsze jest zagrożony robić gorsze rzeczy ...

Z oryginalnego postu

void printLine(const string & line) {
  cout << line << endl;
}

Jeśli jesteś wystarczająco pedantyczny, możesz zauważyć, że printLine nadal robi dwie rzeczy: zapisuje linię do cout i dodaje znak „linii końcowej”. Niektóre osoby mogą chcieć sobie z tym poradzić, tworząc nowe funkcje:

void printLine(const string & line) {
  reallyPrintLine(line);
  addEndLine();
}

void reallyPrintLine(const string & line) {
  cout << line;
}

void addEndLine() {
  cout << endl;
}

O nie, teraz problem jeszcze się pogorszył! Teraz jest nawet oczywiste, że printLine robi DWIE rzeczy !!! 1! Nie jest głupotą tworzenie najbardziej absurdalnych „obejść”, jakie można sobie wyobrazić, aby pozbyć się tego nieuchronnego problemu polegającego na tym, że wydrukowanie linii polega na wydrukowaniu samej linii i dodaniu znaku końca linii.

void printLine(const string & line) {
  for (int i=0; i<2; i++)
    reallyPrintLine(line, i);
}

void reallyPrintLine(const string & line, int action) {
  cout << (action==0?line:endl);
}

Krótka odpowiedź ... to zależy.

Pomyśl o tym: co jeśli w przyszłości nie będziesz chciał drukować tylko na standardowym wyjściu, ale na pliku.

Wiem, co to jest YAGNI, ale mówię tylko, że mogą istnieć przypadki, w których niektóre implementacje są potrzebne, ale odłożone. Więc może architekt lub cokolwiek wie, że ta funkcja musi być w stanie drukować również do pliku, ale nie chce teraz wykonać implementacji. Więc tworzy tę dodatkową funkcję, więc w przyszłości wystarczy zmienić wynik tylko w jednym miejscu. Ma sens?

Jeśli jednak jesteś pewien, że potrzebujesz tylko wyjścia w konsoli, to nie ma większego sensu. Pisanie „otoki” cout <<wydaje się bezużyteczne.

Cały powód, dla którego istnieją książki poświęcające rozdziały cnotom „rób jedną rzecz”, polega na tym, że wciąż istnieją programiści, którzy piszą funkcje o długości 4 stron i zagnieżdżają warunki warunkowe na 6 poziomach. Jeśli Twój kod jest prosty i przejrzysty, zrobiłeś to dobrze.

Jak skomentowali inni plakaty, zrobienie jednej rzeczy jest kwestią skali.

Sugerowałbym również, że pomysłem One Thing jest powstrzymanie ludzi kodujących na skutek efektów ubocznych. Przykładem tego jest sekwencyjne sprzężenie, w którym metody muszą być wywoływane w określonej kolejności, aby uzyskać „właściwy” wynik.