Kontekst
W Clean Code , strona 35, to mówi
Oznacza to, że bloki w instrukcjach if, instrukcjach else, instrukcjach while i tak dalej powinny mieć długość jednego wiersza. Prawdopodobnie ta linia powinna być wywołaniem funkcji. To nie tylko sprawia, że funkcja zamykająca jest mała, ale także dodaje wartość dokumentalną, ponieważ funkcja wywoływana w bloku może mieć ładnie opisową nazwę.
Zgadzam się całkowicie, to ma sens.
Później, na stronie 40, mówi o argumentach funkcji
Idealna liczba argumentów dla funkcji wynosi zero (niladic). Dalej jest jeden (monadyczny), a następnie dwa (dyadyczny). Tam, gdzie to możliwe, należy unikać trzech argumentów (triadycznych). Więcej niż trzy (poliady) wymagają bardzo specjalnego uzasadnienia - i i tak nie powinny być używane. Argumenty są trudne. Biorą dużo mocy pojęciowej.
Zgadzam się całkowicie, to ma sens.
Kwestia
Jednak często zdarza mi się tworzyć listę z innej listy i będę musiał żyć z jednym z dwóch złych.
Albo używam dwóch wierszy w bloku , jeden do stworzenia rzeczy, jeden do dodania go do wyniku:
public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
List<Flurp> flurps = new List<Flurp>();
foreach (BadaBoom badaBoom in badaBooms)
{
Flurp flurp = CreateFlurp(badaBoom);
flurps.Add(flurp);
}
return flurps;
}
Lub dodaję argument do funkcji listy, do której rzecz zostanie dodana, co czyni go „jednym argumentem gorszym”.
public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
List<Flurp> flurps = new List<Flurp>();
foreach (BadaBoom badaBoom in badaBooms)
{
CreateFlurpInList(badaBoom, flurps);
}
return flurps;
}
Pytanie
Czy są jakieś (nie) zalety, których nie widzę, które sprawiają, że jedna z nich jest ogólnie lepsza? Czy są takie zalety w niektórych sytuacjach; w takim przypadku, na co powinienem zwrócić uwagę przy podejmowaniu decyzji?
źródło
flurps.Add(CreateFlurp(badaBoom));
?f(g(x))
jest sprzeczne z twoim przewodnikiem po stylu, cóż, nie mogę naprawić twojego przewodnika po stylu. To znaczy, ty też nie dzielisz sięsqrt(x*x + y*y)
na cztery linie, prawda? I to trzy (!) Zagnieżdżone podwyrażenia na dwóch (!) Wewnętrznych poziomach zagnieżdżenia (wstrzymanie oddechu!). Twoim celem powinna być czytelność , a nie oświadczenia jednego operatora. Jeśli chcesz później, cóż, mam dla Ciebie idealny język: asembler.mov
instrukcji x86 i jednegojmp toStart
na końcu. Ktoś faktycznie stworzył kompilator, który robi dokładnie to: Drlwimi
instrukcji dotyczącej PPC. (To znaczy Obróć lewe słowo Natychmiastowe wstawienie maski). Polecenie to zajęło nie mniej niż pięć operandów (dwa rejestry i trzy bezpośrednie wartości) i wykonało następujące operacje: Zawartość jednego rejestru została obrócona przez natychmiastowe przesunięcie, maska była utworzony za pomocą pojedynczego przebiegu 1 bitów, który był kontrolowany przez dwa inne bezpośrednie argumenty, a bity odpowiadające 1 bitowi w tej masce w drugim argumencie rejestru zostały zastąpione odpowiednimi bitami obróconego rejestru. Bardzo fajna instrukcja :-)Odpowiedzi:
Te wytyczne to kompas, a nie mapa. Wskazują ci rozsądny kierunek . Ale tak naprawdę nie mogą powiedzieć ci absolutnie, które rozwiązanie jest „najlepsze”. W pewnym momencie musisz przestać iść w kierunku wskazywanym przez kompas, ponieważ dotarłeś do celu.
Clean Code zachęca do dzielenia kodu na bardzo małe, oczywiste bloki. To ogólnie dobry kierunek. Ale jeśli dojdziemy do skrajności (jak sugeruje dosłowna interpretacja cytowanej porady), wówczas podzielimy kod na bezużyteczne małe części. Nic tak naprawdę nic nie robi, wszystko tylko deleguje. Jest to zasadniczo inny rodzaj zaciemnienia kodu.
Twoim zadaniem jest zrównoważyć „mniejszy jest lepszy” z „zbyt mały jest bezużyteczny”. Zadaj sobie pytanie, które rozwiązanie jest prostsze. Dla mnie jest to oczywiście pierwsze rozwiązanie, ponieważ oczywiście tworzy listę. To dobrze rozumiany idiom. Można zrozumieć ten kod bez konieczności patrzenia na jeszcze jedną funkcję.
Jeśli można to zrobić lepiej, należy zauważyć, że „przekształcenie wszystkich elementów z listy na inną listę” jest powszechnym wzorcem, który często można wyabstrahować za pomocą
map()
operacji funkcjonalnej . Myślę, że w C # to się nazywaSelect
. Coś takiego:źródło
CreateFlurps(someList)
skoro BCL już zapewniasomeList.ConvertAll(CreateFlurp)
?BadaBoom => CreateFlurp(badaBoom)
jest zbędny; możesz przekazaćCreateFlurp
jako funkcję bezpośrednio (Select(CreateFlurp)
). (O ile mi wiadomo, zawsze tak było).CreateFlurps
jest w rzeczywistości bardziej myląca i trudniejsza do zrozumienia niż tylko widzeniebadaBooms.Select(CreateFlurp)
. Ta ostatnia jest całkowicie deklaratywna - nie ma problemu z rozkładem, a zatem nie ma potrzeby stosowania żadnej metody.badaBooms.Select(CreateFlurp)
. Tworzysz metodę, aby jej nazwa (wysoki poziom) oznaczała jej implementację (niski poziom). W tym przypadku są na tym samym poziomie, więc aby dowiedzieć się dokładnie, co się dzieje, muszę po prostu przyjrzeć się metodzie (zamiast widzieć ją w linii).CreateFlurps(badaBooms)
może mieć niespodzianki, alebadaBooms.Select(CreateFlurp)
nie może. Jest także mylące, ponieważ błędnie prosi oList
zamiastIEnumerable
.Nie! Idealna liczba argumentów dla funkcji to jeden. Jeśli wynosi zero, to gwarantujesz, że funkcja musi uzyskać dostęp do informacji zewnętrznych, aby móc wykonać akcję. „Wujek” Bob bardzo źle to zrozumiał.
Jeśli chodzi o kod, twój pierwszy przykład ma tylko dwa wiersze w bloku, ponieważ tworzysz zmienną lokalną w pierwszym wierszu. Usuń to zadanie, a postępujesz zgodnie z tymi wytycznymi dotyczącymi czystego kodu:
Ale to jest bardzo długi kod (C #). Po prostu zrób to jako:
źródło
Porada dotycząca „czystego kodu” jest całkowicie błędna.
Użyj dwóch lub więcej linii w swojej pętli. Ukrywanie tych samych dwóch linii w funkcji ma sens, gdy są to losowe matematyki, które wymagają opisu, ale nic nie robi, gdy linie są już opisowe. „Utwórz” i „Dodaj”
Druga metoda, o której wspominasz, nie ma żadnego sensu, ponieważ nie musisz dodawać drugiego argumentu, aby uniknąć dwóch linii.
lub
Jak zauważyli inni, rada, że najlepszą funkcją jest ta bez argumentów, jest w najlepszym razie wypaczona dla OOP, aw najgorszym przypadku zwykła zła rada
źródło
Drugi jest zdecydowanie gorszy, ponieważ
CreateFlurpInList
akceptuje listę i modyfikuje tę listę, przez co funkcja nie jest czysta i trudniejsza do uzasadnienia. Nic w nazwie metody nie sugeruje, że metoda dodaje tylko do listy.I oferuję trzecią, najlepszą opcję:
I do diabła, możesz natychmiast wprowadzić tę metodę, jeśli jest tylko jedno miejsce, w którym jest używana, ponieważ jednowarstwowa jest sama w sobie czysta, więc nie musi być enkapsulowana metodą, aby nadać jej znaczenie.
źródło
Wersja z jednym argumentem jest lepsza, ale nie przede wszystkim ze względu na liczbę argumentów.
Najważniejszym powodem, dla którego lepiej jest to, że ma niższe sprzężenie , co czyni go bardziej użytecznym, łatwiejszym do przemyślenia, łatwiejszym do przetestowania i mniejszym prawdopodobieństwem przekształcenia się w kopiowanie + wklejanie klonów.
Jeśli dostarczyć mi
CreateFlurp(BadaBoom)
, że mogę korzystać z każdego rodzaju pojemnika zbiorczego: ProstyFlurp[]
,List<Flurp>
,LinkedList<Flurp>
,Dictionary<Key, Flurp>
, i tak dalej. Ale zCreateFlurpInList(BadaBoom, List<Flurp>)
, wracam do ciebie jutro z prośbąCreateFlurpInBindingList(BadaBoom, BindingList<Flurp>)
, aby mój model mógł uzyskać powiadomienie o zmianie listy. Fuj!Dodatkową korzyścią jest to, że prostszy podpis bardziej pasuje do istniejących interfejsów API. Mówisz, że masz powtarzający się problem
To tylko kwestia użycia dostępnych narzędzi. Najkrótsza, najbardziej wydajna i najlepsza wersja to:
Jest to nie tylko mniej kodu do pisania i testowania, ale także szybsze, ponieważ
List<T>.ConvertAll()
jest wystarczająco inteligentny, aby wiedzieć, że wynik będzie miał taką samą liczbę elementów, jak dane wejściowe, i wstępnie przydzielić listę wyników do odpowiedniego rozmiaru. Podczas gdy twój kod (obie wersje) wymagał powiększenia listy.źródło
List.ConvertAll
. Nazywany jest idiomatyczny sposób mapowania obiektów policzalnych na różne obiekty w języku C #Select
. Jedyny powódConvertAll
jest tutaj dostępny nawet dlatego, że OP błędnie prosi oList
metodę - powinna byćIEnumerable
.Pamiętaj o ogólnym celu: ułatwieniu odczytu i utrzymania kodu.
Często możliwe jest grupowanie wielu linii w jedną znaczącą funkcję. Zrób to w tych przypadkach. Czasami trzeba ponownie rozważyć swoje ogólne podejście.
Na przykład w twoim przypadku zastąpienie całej implementacji var
może być możliwe. Lub możesz zrobić coś takiego
Czasami najczystsze i najbardziej czytelne rozwiązanie po prostu nie mieści się w jednej linii. Więc będziesz miał dwie linie. Nie utrudniaj zrozumienia kodu, wystarczy spełnić dowolną regułę.
Twój drugi przykład jest (moim zdaniem) znacznie trudniejszy do zrozumienia niż pierwszy. Nie chodzi tylko o to, że masz drugi parametr, ale o to, że parametr jest modyfikowany przez funkcję. Sprawdź, co ma do powiedzenia Clean Code. (Nie mam w tej chwili książki pod ręką, ale jestem prawie pewien, że w zasadzie „nie rób tego, jeśli możesz tego uniknąć”).
źródło