Czy podzielić obliczenia wartości zwracanej i instrukcji return na metody jednowierszowe?

Rozmawiałem ze współpracownikiem na temat złamania returninstrukcji i instrukcji, która oblicza wartość zwracaną w dwóch wierszach.

Na przykład

private string GetFormattedValue()
{
    var formattedString = format != null ? string.Format(format, value) : value.ToString();
    return formattedString;
}

zamiast

private string GetFormattedValue()
{
    return format != null ? string.Format(format, value) : value.ToString();
}

Pod względem kodowym tak naprawdę nie widzę wartości w pierwszym wariancie. Dla mnie ta ostatnia jest bardziej przejrzysta, szczególnie w przypadku tak krótkich metod. Jego argumentem było to, że poprzedni wariant jest łatwiejszy do debugowania - co jest dość małą zaletą, ponieważ VisualStudio pozwala nam na bardzo szczegółową kontrolę instrukcji, gdy wykonanie jest zatrzymane z powodu punktu przerwania.

Moje pytanie brzmi: czy nadal jest poprawny punkt do pisania kodu mniej wyraźnego, tylko po to, aby ułatwić debugowanie rzutu oka? Czy są jakieś inne argumenty przemawiające za wariantem z kalkulacją i returnwyciągiem podzielonym ?

Wprowadzenie zmiennych objaśniających jest dobrze znanym refaktoryzowaniem, które może czasem pomóc w zwiększeniu czytelności skomplikowanych wyrażeń. Jednak w pokazanym przypadku

• dodatkowa zmienna nie „wyjaśnia” niczego, co nie jest jasne z otaczającej nazwy metody
• instrukcja staje się jeszcze dłuższa, więc (nieco) mniej czytelna

Co więcej, nowsze wersje debugera Visual Studio mogą pokazywać wartość zwracaną przez funkcję w większości przypadków bez wprowadzania zbędnej zmiennej (ale uwaga, są pewne zastrzeżenia, spójrz na ten starszy post SO i różne odpowiedzi ).

Tak więc w tym konkretnym przypadku zgadzam się z tobą, jednak istnieją inne przypadki, w których zmienna wyjaśniająca może rzeczywiście poprawić jakość kodu.

Biorąc pod uwagę fakty, które:

a) Nie ma wpływu na końcowy kod, ponieważ kompilator optymalizuje zmienną.

b) Oddzielne rozszerzenie zwiększa możliwości debugowania.

Osobiście doszedłem do wniosku, że dobrą praktyką jest ich rozdzielanie w 99% przypadków.

Robienie tego w ten sposób nie ma istotnych wad. Argument, że przesadza kod, jest błędny, ponieważ rozdęty kod jest trywialnym problemem w porównaniu z nieczytelnym lub trudnym do debugowania kodem. Co więcej, ta metoda sama w sobie nie może tworzyć mylącego kodu, co całkowicie zależy od programisty.

Często wprowadzenie zmiennej tylko w celu nazwania jakiegoś wyniku jest bardzo pomocne, gdy sprawia, że ​​kod jest bardziej samodokumentujący. W tym przypadku nie jest to czynnik, ponieważ nazwa zmiennej jest bardzo podobna do nazwy metody.

Zauważ, że metody jednej linii nie mają żadnej wartości nieodłącznej. Jeśli zmiana wprowadza więcej wierszy, ale kod jest jaśniejszy, to dobra zmiana.

Ale ogólnie rzecz biorąc, decyzje te są wysoce zależne od osobistych preferencji. Np. Uważam oba rozwiązania za mylące, ponieważ operator warunkowy jest używany niepotrzebnie. Wolałbym instrukcję if. Ale w swoim zespole mogłeś zgodzić się na różne konwencje. Następnie rób wszystko, co sugerują twoje konwencje. Jeśli konwencje milczą w takim przypadku, zauważ, że jest to bardzo niewielka zmiana, która nie ma znaczenia na dłuższą metę. Jeśli ten wzorzec występuje wielokrotnie, być może zainicjuj dyskusję na temat tego, jak Ty jako zespół chcesz poradzić sobie z tymi przypadkami. Ale to dzieli włosy na „dobry kod” i „może odrobinę lepszy kod”.

W odpowiedzi na twoje pytania:

Moje pytanie brzmi: czy nadal jest poprawny punkt do pisania kodu mniej wyraźnego, tylko po to, aby ułatwić debugowanie rzutu oka?

Tak. W rzeczywistości część twojego wcześniejszego stwierdzenia wydaje mi się (bez obrazy) trochę krótkowzroczna (patrz pogrubienie poniżej). „ Jego argumentem było to, że poprzedni wariant jest łatwiejszy do debugowania - co jest dość małą zaletą , ponieważ VisualStudio pozwala nam na bardzo szczegółową kontrolę wyciągów, gdy wykonanie jest zatrzymane z powodu punktu przerwania. ”

Ułatwienie debugowania nigdy (prawie) nigdy nie ma „ małej wartości ”, ponieważ według niektórych szacunków 50% czasu programisty spędza na debugowaniu ( Reversible Debugging Software ).

Czy są jakieś dalsze argumenty dla wariantu z obliczeniem podziału i instrukcją return?

Tak. Niektórzy programiści twierdzą, że obliczenia podziału są łatwiejsze do odczytania. To oczywiście pomaga w debugowaniu, ale także pomaga, gdy ktoś próbuje odszyfrować wszelkie reguły biznesowe, które Twój kod może wykonać lub zastosować.

UWAGA: Reguły biznesowe mogą być lepiej obsługiwane w bazie danych, ponieważ mogą się często zmieniać. Niemniej jednak jasne kodowanie w tym obszarze jest nadal najważniejsze. ( Jak zbudować silnik reguł biznesowych )

Poszedłbym jeszcze dalej:

private string GetFormattedValue()
{
    if (format != null) {
        formattedString = string.Format(format, value);
    } else {
        formattedString = value.ToString()
    }
    return formattedString;
}

Czemu?

Używanie operatorów trójskładnikowych do bardziej skomplikowanej logiki byłoby nieczytelne, więc można użyć stylu podobnego do powyższego w przypadku bardziej złożonych instrukcji. Używając zawsze tego stylu, kod jest spójny i łatwiejszy do przeanalizowania przez człowieka. Dodatkowo, wprowadzając tego rodzaju spójność (i używając wskazówek do kodu i innych testów) możesz uniknąć goto failbłędów pisowni .

Kolejną zaletą jest to, że raport pokrycia kodu poinformuje cię, jeśli zapomniałeś dołączyć test dla formatnie jest zerowy. Nie byłoby tak w przypadku operatora trójskładnikowego.

Moja preferowana alternatywa - jeśli jesteś w tłumie „uzyskaj jak najszybszy zwrot”, a nie przeciwko wielokrotnym zwrotom z metody:

private string GetFormattedValue()
{
    if (format != null) {
        return string.Format(format, value);
    }

    return value.ToString();
}

Możesz więc spojrzeć na ostatni powrót, aby zobaczyć, jaka jest wartość domyślna.

Ważne jest, aby być konsekwentnym - i aby wszystkie metody były zgodne z jedną lub drugą konwencją.

Nie sądzę, że taką technikę można uzasadnić potrzebą debugowania. Sam spotkałem się z tym podejściem tysiąc razy i od czasu do czasu to robię, ale zawsze pamiętam, co Martin Fowler powiedział o debugowaniu :

Ludzie również nie doceniają czasu spędzanego na debugowaniu. Nie doceniają czasu, jaki mogą spędzić, ścigając długi błąd. Dzięki testom od razu wiem, kiedy dodałem błąd. To pozwala mi natychmiast naprawić błąd, zanim będzie mógł się czołgać i ukryć. Jest kilka rzeczy bardziej frustrujących lub marnujących czas niż debugowanie. Czy nie byłoby o wiele szybciej, gdybyśmy po prostu nie stworzyli błędów?

Myślę, że niektórzy ludzie rozwieszają się w kwestiach związanych z pytaniem, takich jak operator trójskładnikowy. Tak, wiele osób go nienawidzi, więc może i tak warto to wychowywać.

Jeśli chodzi o temat pytania, przeniesienie zwróconej instrukcji do odniesienia przez zmienną ...

To pytanie zawiera 2 założenia, z którymi się nie zgadzam:

1. Że drugi wariant jest bardziej przejrzysty lub czytelny (mówię, że jest odwrotnie), i

2. że wszyscy używają Visual Studio. Korzystałem z programu Visual Studio wiele razy i mogę go używać dobrze, ale zwykle używam czegoś innego. Sceptyczne jest środowisko deweloperskie, które wymusza określone IDE.

Podział czegoś na nazwaną zmienną rzadko sprawia, że ​​coś jest trudniejsze do odczytania, prawie zawsze robi to odwrotnie. Specyficzny sposób, w jaki ktoś to robi, może powodować problemy, na przykład, jeśli robi var thisVariableIsTheFormattedResultAndWillBeTheReturnValue = ...to sam dokumentalista, to oczywiście źle, ale to osobny problem. var formattedText = ...jest w porządku.

W tym konkretnym przypadku i prawdopodobnie w wielu przypadkach, ponieważ mówimy o liniach 1-liniowych, zmienna nie powiedziałaby wiele, że nazwa funkcji jeszcze nie mówi. Dlatego zmienna nie dodaje tyle. Argument debugowania nadal może się utrzymywać, ale ponownie, w tym konkretnym przypadku nie widzę niczego, co mogłoby być twoim celem podczas debugowania, i zawsze można go łatwo zmienić później, jeśli ktoś potrzebuje tego formatu do debugowania lub cokolwiek innego.

Ogólnie rzecz biorąc, i poprosiłeś o ogólną zasadę (twój przykład był po prostu przykładem formy uogólnionej), wszystkie punkty przedstawione na korzyść wariantu 1 (2-liniowego) są poprawne. Są to dobre wytyczne. Ale wytyczne muszą być elastyczne. Na przykład projekt, nad którym pracuję, ma teraz maksymalnie 80 znaków na linię, więc dzielę wiele linii, ale zwykle znajduję linie 81-85 znaków, które trudno byłoby podzielić lub zmniejszyć czytelność i pozostawiam je ponad limit.

Ponieważ jest mało prawdopodobne, aby dodać wartość, nie zrobiłbym 2 linii dla podanego konkretnego przykładu. Zrobiłbym wariant 2 (1-liniowy), ponieważ punkty nie są wystarczająco mocne, aby zrobić inaczej w tym przypadku.