Dodatkowa linia w bloku vs dodatkowy parametr w Clean Code

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?

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ę nazywa Select. Coś takiego:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(BadaBoom => CreateFlurp(badaBoom)).ToList();
}

Idealna liczba argumentów dla funkcji wynosi zero (niladic)

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:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    List<Flurp> flurps = new List<Flurp>();
    foreach (BadaBoom badaBoom in badaBooms)
    {
        flurps.Add(CreateFlurp(badaBoom));
    }
    return flurps;
}

Ale to jest bardzo długi kod (C #). Po prostu zrób to jako:

IEnumerable<Flurp> CreateFlurps(IEnumerable<BadaBoom> badaBooms) =>
    from badaBoom in babaBooms select CreateFlurp(badaBoom);

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.

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
    {
        List<Flurp> flurps = new List<Flurp>();
        foreach (BadaBoom badaBoom in badaBooms)
        {
            flurps.Add(badaBoom .CreateFlurp());
            //or
            badaBoom.AddToListAsFlurp(flurps);
            //or
            flurps.Add(new Flurp(badaBoom));
            //or
            //make flurps a member of the class
            //use linq.Select()
            //etc
        }
        return flurps;
    }

lub

foreach(var flurp in ConvertToFlurps(badaBooms))...

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

Drugi jest zdecydowanie gorszy, ponieważ CreateFlurpInListakceptuje 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ę:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(CreateFlurp).ToList();
}

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.

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: Prosty Flurp[], List<Flurp>, LinkedList<Flurp>, Dictionary<Key, Flurp>, i tak dalej. Ale z CreateFlurpInList(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

raczej często zdarza mi się tworzyć listę z innej listy

To tylko kwestia użycia dostępnych narzędzi. Najkrótsza, najbardziej wydajna i najlepsza wersja to:

var Flurps = badaBooms.ConvertAll(CreateFlurp);

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.

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

flups = badaBooms.Select(bb => new Flurp(bb));

może być możliwe. Lub możesz zrobić coś takiego

flups.Add(new Flurp(badaBoom))

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ąć”).