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

26

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 ?

coding-style code-quality debugging

— Paul Kertscher
źródło

18

Nie działa na VS, ale zakłada, że nie można ustawić warunkowego punktu przerwania dla skomplikowanego wyrażenia (lub byłoby trudne wprowadzenie), więc prawdopodobnie dla wygody umieściłby przypisać i powrócić do oddzielnych instrukcji. Kompilator najprawdopodobniej wymyśliby ten sam kod dla obu.

— tofro

1

Jest to prawdopodobnie zależne od języka, szczególnie w językach, w których zmienne mają (prawdopodobnie złożone) zachowanie obiektu zamiast zachowania wskaźnika. Oświadczenie Pawła K jest prawdopodobnie prawdziwe w przypadku języków z zachowaniem wskaźnika, języków, w których obiekty mają proste zachowanie wartości, oraz języków z dojrzałymi kompilatorami wysokiej jakości.

— MSalters,

4

„ponieważ VisualStudio pozwala nam na bardzo szczegółową kontrolę instrukcji, kiedy wykonanie jest zatrzymane z powodu punktu przerwania” - tak jest. Jak więc uzyskać wartość zwracaną, jeśli funkcja zwróciła strukturę z więcej niż jednym elementem? (a obsługa tej funkcji jest w najlepszym razie nierówna, istnieje wiele kombinacji, w których nie uzyskuje się w ogóle wartości zwrotu).

— Voo,

2

W jaki sposób „bardzo szczegółowa kontrola instrukcji” w debugerze, którego ktoś używa DZISIAJ, sprawia, że pisanie kodu jest złe, więc debugowanie w KAŻDYM debugerze jest łatwe?

— Ian

16

Zirytuj go jeszcze bardziej, redukując całe ciało funkcyjne doprivate string GetFormattedValue() => string.Format(format ?? "{0}", value);

— Graham

46

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.

— Doktor Brown
źródło

Zgadzam się również, że zdecydowanie są przypadki, w których jest to bez wątpienia przydatne.

— Paul Kertscher,

2

Zwykle używam resultjako nazwy zmiennej. Nie tyle dłużej i łatwiej debugować

— edc65

26

@ edc65: ogólna nazwa, taka jak result często, powoduje po prostu szum w kodzie i rzadko zwiększa czytelność, co jest dokładnie celem mojej odpowiedzi. Można to usprawiedliwić w kontekście, w którym pomaga to w debugowaniu, ale unikałbym tego podczas korzystania z debugera, który nie potrzebuje oddzielnej zmiennej.

— Dok. Brown

6

@JonHanna droga narzędzie długo według mnie. Nazwa resultprzekazuje informację, że jest to wartość wynikająca z funkcji, dzięki czemu można na nią spojrzeć przed powrotem funkcji.

— edc65

1

@ edc65, ale dzięki temu wygląda na użyteczny. Więc teraz, kiedy czytam twój kod, nie od razu zdaję sobie sprawę, że nie jest. Twój kod stał się mniej czytelny.

— Jon Hanna,

38

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.

— Dom
źródło

9

To jest dla mnie poprawna odpowiedź. Ułatwia to ustawienie punktu przerwania i zobaczenie wartości podczas debugowania i nie ma żadnych wad, o których jestem świadomy.

— Matthew James Briggs,

W przypadku punktu b w programie Visual Studio Code po prostu umieść punkt zwrotny na powrocie, a następnie dodaj wyrażenie: GetFormattedValue (), które wyświetli wynik po trafieniu punktu przerwania, więc dodatkowa linia nie jest potrzebna. Ale łatwiej jest zobaczyć miejscowych z dodatkową linią, ponieważ nie będzie wymagało dodawania żadnego dodatkowego wyrażenia w debuggerze. Tak naprawdę to kwestia osobistych preferencji.

— Jon Raynor,

3

@JonRaynor dla wartości zwracanej, umieść punkt przerwania na zamykającym nawiasie funkcji. Przechwytuje zwracaną wartość, nawet w funkcjach z wieloma zwrotami.

— Baldrickk,

16

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”.

— amon
źródło

1

„Uważam oba rozwiązania za mylące, ponieważ operator warunkowy jest używany niepotrzebnie”. - To nie jest przykład z prawdziwego świata, po prostu musiałem coś szybko wymyślić. Trzeba przyznać, że nie jest to najlepszy przykład.

— Paul Kertscher,

4

+1 za zasadniczo powiedzenie, że jest to różnica „pod radarem”, że (inne rzeczy są równe) nie warto się tym przejmować.

— TripeHound,

3

@Mindwin, kiedy używam operatora trójskładnikowego, zwykle dzielę go na wiele wierszy, aby było jasne, co jest prawdą, a co fałszywą.

— Arturo Torres Sánchez,

2

@ ArturoTorresSánchez Też to robię, ale zamiast a ?i :używam if() {i } else {- - - - \\ :)

— Mindwin,

3

@Mindwin, ale nie mogę tego zrobić, gdy jestem w środku wyrażenia (jak inicjator obiektu)

— Arturo Torres Sánchez

2

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 )

— tale852150
źródło

1

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ą.

— HorusKol
źródło

1

Pierwszy przykład wydaje się złą praktyką, ponieważ value.ToString()jest niepotrzebnie wywoływany, gdy format ma wartość inną niż null. W ogólnym przypadku może to obejmować nietrywialne obliczenia i może potrwać dłużej niż wersja zawierająca ciąg formatu. Rozważmy na przykład a, valuektóry przechowuje PI do miliona miejsc po przecinku, i ciąg formatu, który żąda tylko kilku pierwszych cyfr.

— Steve,

1

dlaczego nie private string GetFormattedValue() => string.Format(format ?? "{0}", value); Ten sam wpływ i użyj testów jednostkowych, aby zapewnić poprawność, zamiast polegać na debuggerze.

— Berin Loritsch,

1

Chociaż zgadzam się, że trójka może być mniej wyraźna, terminator zerowy może sprawić, że wszystko będzie bardziej jasne. Przynajmniej w tym przypadku.

— Berin Loritsch,

1

Drogi pamiętniku, dzisiaj przeczytałem, że pisanie jasnego i zwięzłego kodu przy użyciu dobrze znanych (istniejących od około 40 lat) paradygmatów, idiomów i operatorów to: cytat, podwójny cytat to sprytny podwójny cytat, bez cudzysłowu - ale zamiast tego piszę zbyt szczegółowy kod naruszający OSUSZANIE i nieużywanie wyżej wymienionych operatorów, idiomów i paradygmatów, podczas gdy zamiast tego próbuje się unikać czegokolwiek, co mogłoby wydawać się tajemnicze tylko dla pięciolatka bez wcześniejszego doświadczenia w programowaniu - jest zamiast tego jasnością . Do diabła, musiałem się naprawdę zestarzeć, mój drogi pamiętniku ... Powinienem był nauczyć się Go, kiedy miałem okazję.

— vaxquis,

1

„Używanie operatorów trójskładnikowych do bardziej skomplikowanej logiki byłoby nieczytelne” Chociaż tak naprawdę jest (i widziałem, jak ludzie nadmiernie komplikują logikę), nie jest to prawdą w przypadku kodu OP, a także nie jest czymś szczególnym dla operatorów trójskładnikowych. Jedyne, co mogę powiedzieć z pełnym przekonaniem, to to, że linia jest za długa. gist.github.com/milleniumbug/cf9b62cac32a07899378feef6c06c776 to sposób, w jaki sformatowałbym to.

— milleniumbug

1

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?

— Zapadło
źródło

Martin Fowler jest inteligentnym człowiekiem i podobało mi się czytanie jego (i twoich) poglądów. Chociaż mocno wierzę, że testowanie jest konieczne i że należy poświęcić więcej czasu na to przedsięwzięcie, fakt, że wszyscy jesteśmy omylnymi ludźmi sugeruje, że żadna ilość testów nie wyeliminuje wszystkich błędów. W związku z tym debugowanie zawsze będzie częścią procesu opracowywania i obsługi programu.

— tale852150,

1

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:

Że drugi wariant jest bardziej przejrzysty lub czytelny (mówię, że jest odwrotnie), i
ż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.

— Aaron
źródło