Podaj opcjonalne nazwy parametrów, nawet jeśli nie są wymagane?

25

Rozważ następującą metodę:

public List<Guid> ReturnEmployeeIds(bool includeManagement = false)
{

}

I następujące połączenie:

var ids = ReturnEmployeeIds(true);

Dla programistów, którzy są nowicjuszami w systemie, odgadnięcie, co się stało, byłoby dość trudne true. Najpierw najedź kursorem na nazwę metody lub przejdź do definicji (z których żadne nie jest dużym zadaniem). Ale dla czytelności sensowne jest napisanie:

var ids = ReturnEmployeeIds(includeManagement: true);

Czy jest gdzieś, gdzie formalnie dyskutuje się, czy jawnie określić parametry opcjonalne, gdy kompilator nie wymaga tego?

W poniższym artykule omówiono niektóre konwencje kodowania: https://msdn.microsoft.com/en-gb/library/ff926074.aspx

Coś podobnego do powyższego artykułu byłoby świetne.

JᴀʏMᴇᴇ
źródło
2
Są to dwa raczej niezwiązane pytania: (1) Czy powinieneś napisać opcjonalne argumenty za jasnością? (2) Czy należy używać booleanargumentów metod i jak?
Kilian Foth,
5
Wszystko sprowadza się do osobistych preferencji / stylu / opinii. Osobiście lubię nazwy parametrów, gdy przekazywana wartość jest dosłowna (zmienna może już przekazywać wystarczającą ilość informacji poprzez swoją nazwę). Ale to moja osobista opinia.
MetaFight,
1
może podać ten link jako przykładowe źródło w swoim pytaniu?
MetaFight,
4
Jeśli coś dodaje, uruchomiłem to przez StyleCop i ReSharper - żadne z nich nie miało na to żadnego wyraźnego widoku. ReSharper po prostu mówi, że nazwany parametr można usunąć, a następnie mówi, że można dodać nazwany parametr . Porady są bezpłatne z jakiegoś powodu ...: - /
Robbie Dee,

Odpowiedzi:

28

Powiedziałbym, że w świecie C # wyliczenie byłoby jedną z najlepszych opcji tutaj.

Dzięki temu będziesz musiał przeliterować swoje działania, a każdy użytkownik zobaczy, co się dzieje. Jest również bezpieczny dla przyszłych rozszerzeń;

public enum WhatToInclude
{
    IncludeAll,
    ExcludeManagement
}

var ids = ReturnEmployeeIds(WhatToInclude.ExcludeManagement);

Moim zdaniem tutaj:

enum> opcjonalne enum> opcjonalne bool

Edycja: Ze względu na dyskusję z LeopardSkinPillBoxHat poniżej dotyczącą [Flags]wyliczenia, które w tym konkretnym przypadku może być dobre dopasowanie (ponieważ konkretnie mówimy o włączaniu / wyłączaniu rzeczy), zamiast tego proponuję użyć ISet<WhatToInclude>jako parametru.

Jest to bardziej „nowoczesna” koncepcja z kilkoma zaletami, wynikającymi głównie z faktu, że pasuje do rodziny kolekcji LINQ, ale [Flags]ma także maksymalnie 32 grupy. Główną wadą używania ISet<WhatToInclude>jest to, jak źle obsługiwane są składnie w C #:

var ids = ReturnEmployeeIds(
    new HashSet<WhatToInclude>(new []{ 
        WhatToInclude.Cleaners, 
        WhatToInclude.Managers});

Niektóre z nich mogą zostać złagodzone przez funkcje pomocnicze, zarówno do generowania zestawu, jak i tworzenia „zestawów skrótów”, takich jak

var ids = ReturnEmployeeIds(WhatToIncludeHelper.IncludeAll)
NiklasJ
źródło
Zwykle się zgadzam; i ogólnie zacząłem używać wyliczeń w moim nowym kodzie, jeśli istnieje jakakolwiek szansa, że ​​warunki zostaną rozszerzone w przyszłości. Zastąpienie wartości logicznej wyliczeniem, gdy coś staje się stanem trójstanowym, powoduje naprawdę głośne różnice i sprawia, że ​​obwinianie kodu jest o wiele bardziej nużące; kiedy w końcu będziesz musiał ponownie wprowadzić kod w celu uzyskania trzeciej opcji.
Dan Neely,
3
I podobnie jak enumpodejście, ale nie jestem wielkim fanem mieszania Includei Excludew taki sam enum, jak to jest trudniejsze do uchwycenia tego, co się dzieje. Jeśli chcesz, aby było to naprawdę rozszerzalne, możesz uczynić to a [Flags] enum, uczynić je wszystkimi IncludeXxxi zapewnić ustawienie, IncludeAllktóre jest ustawione na Int32.MaxValue. Następnie możesz podać 2 ReturnEmployeeIdsprzeciążenia - jeden, który zwraca wszystkich pracowników, i drugi, który przyjmuje WhatToIncludeparametr filtru, który może być kombinacją flag wyliczeniowych.
LeopardSkinPillBoxHat
@LeopardSkinPillBoxHat Ok, zgadzam się na wyłączenie / włączenie. To jest trochę wymyślony przykład, ale mógłbym nazwać go IncludeAllButManagement. Z drugiej strony, nie zgadzam się - myślę, że [Flags]to relikt i powinien być używany tylko ze względu na dziedzictwo lub wydajność. Myślę, że a ISet<WhatToInclude>jest znacznie lepszą koncepcją (niestety w C # brakuje cukru syntaktycznego, aby był przyjemny w użyciu).
NiklasJ
@NiklasJ - Podoba mi się to Flagspodejście, ponieważ pozwala dzwoniącemu na przejście WhatToInclude.Managers | WhatToInclude.Cleanersi bitowe lub wyraża dokładnie, w jaki sposób dzwoniący je przetworzy (tj. Menedżerowie lub sprzątacze). Intuicyjny cukier syntaktyczny :-)
LeopardSkinPillBoxHat 20.01.2016
@LeopardSkinPillBoxHat Dokładnie, ale to jedyny plus tej metody. Wady obejmują na przykład ograniczoną liczbę opcji i brak zgodności z innymi koncepcjami podobnymi do linq.
NiklasJ
20

czy warto pisać:

 var ids=ReturnEmployeeIds(includeManagement: true);

Jest to dyskusyjne, jeśli jest to „dobry styl”, ale IMHO nie jest co najmniej złym stylem i jest użyteczne, o ile linia kodu nie staje się „zbyt długa”. Bez nazwanych parametrów powszechnym stylem jest również wprowadzanie zmiennej objaśniającej:

 bool includeManagement=true;
 var ids=ReturnEmployeeIds(includeManagement);

Ten styl jest prawdopodobnie bardziej powszechny wśród programistów C # niż nazwany wariant parametru, ponieważ nazwane parametry nie były częścią języka przed wersją 4.0. Wpływ na czytelność jest prawie taki sam, wymaga jedynie dodatkowej linii kodu.

Więc moja rada: jeśli uważasz, że użycie wyliczenia lub dwóch funkcji o różnych nazwach to „przesada” lub nie chcesz lub nie możesz zmienić podpisu z innego powodu, skorzystaj z nazwanego parametru.

Doktor Brown
źródło
Może warto zauważyć, że używasz dodatkowej pamięci na drugim przykładzie
Alvaro
10
@Alvaro Jest to bardzo mało prawdopodobne w tak trywialnym przypadku (gdy zmienna ma stałą wartość). Nawet jeśli tak, to nie jest warte wspomnienia. Większość z nas nie korzysta obecnie z 4KB RAM.
Chris Hayes,
3
Ponieważ jest to C #, możesz oznaczyć includeManagementjako lokalny consti w ogóle unikać używania dodatkowej pamięci.
Jacob Krall,
8
Chłopaki, nie dodam żadnych odpowiedzi do mojej odpowiedzi, które mogą tylko odwrócić uwagę od tego, co uważam za ważne tutaj.
Doc Brown
17

Jeśli napotkasz kod taki jak:

public List<Guid> ReturnEmployeeIds(bool includeManagement = false)
{

}

możesz właściwie zagwarantować, że to, co jest w środku, {}będzie wyglądało tak:

public List<Guid> ReturnEmployeeIds(bool includeManagement = false)
{
    if (includeManagement)
        return something
    else
        return something else
}

Innymi słowy, wartość logiczna jest używana do wyboru między dwoma kierunkami działania: metoda ma dwa obowiązki. Jest to prawie gwarantowany zapach kodu . Zawsze powinniśmy dążyć do tego, aby metody robiły tylko jedną rzecz; ponoszą tylko jedną odpowiedzialność. Dlatego twierdzę, że oba rozwiązania są niewłaściwe. Korzystanie z opcjonalnych parametrów jest znakiem, że metoda robi więcej niż jedną rzecz. Parametry boolowskie są również tego znakiem. Oba powinny zdecydowanie ustawić dzwonki alarmowe. W związku z tym powinieneś podzielić dwa różne zestawy funkcji na dwie osobne metody. Podaj nazwy metod, które wyjaśniają, co robią, np .:

public List<Guid> GetNonManagementEmployeeIds()
{

}

public List<Guid> GetAllEmployeeIds()
{

}

Zarówno poprawia to czytelność kodu, jak i zapewnia lepsze przestrzeganie zasad SOLID.

EDYCJA Jak wskazano w komentarzach, w tym przypadku przypadek, w którym te dwie metody prawdopodobnie będą miały wspólną funkcję, i że ta wspólna funkcjonalność najprawdopodobniej będzie najlepiej zapakowana w funkcję prywatną, która będzie wymagała parametru logicznego do kontrolowania niektórych aspektów tego, co robi .

Czy w takim przypadku należy podać nazwę parametru, nawet jeśli kompilator jej nie potrzebuje? Sugerowałbym, że nie ma prostej odpowiedzi na to pytanie, a wyrok jest potrzebny w poszczególnych przypadkach:

  • Argumentem za podaniem nazwy jest to, że inni programiści (w tym ty za sześć miesięcy) powinni być głównymi odbiorcami twojego kodu. Kompilator jest zdecydowanie drugorzędnym odbiorcą. Dlatego zawsze pisz kod, aby ułatwić innym osobom czytanie; zamiast dostarczać tylko to, czego potrzebuje kompilator. Przykładem tego, jak używamy tej reguły na co dzień, jest to, że nie wypełniamy naszego kodu zmiennymi _1, _2 itd., Używamy znaczących nazw (co dla kompilatora nic nie znaczy).
  • Argument przeciwny jest taki, że metody prywatne są szczegółami implementacji. Można zasadnie oczekiwać, że każdy, kto spojrzy na metodę taką jak poniżej, prześledziłby kod, aby zrozumieć cel parametru boolean, ponieważ znajdują się one w jego wnętrzu:
public List<Guid> GetNonManagementEmployeeIds()
{
    return ReturnEmployeeIds(true);
}

Czy plik źródłowy i metody są małe i łatwe do zrozumienia? Jeśli tak, to nazwany parametr prawdopodobnie oznacza hałas. Jeśli nie, może to być bardzo pomocne. Zastosuj regułę odpowiednio do okoliczności.

David Arno
źródło
12
Słyszę, co mówisz, ale nie rozumiesz sensu tego, o co pytam. Zakładając scenariusz, w którym to jest najbardziej odpowiedni do zastosowania parametry opcjonalne (te scenariusze nie istnieją), to jest ok, aby wymienić parametry nawet jeśli jest to konieczne?
JᴀʏMᴇᴇ
4
Wszystko prawda, ale pytanie dotyczyło wezwania do takiej metody. Również parametr flagi nie gwarantuje wcale gałęzi w metodzie. Może być po prostu przeniesiony na inną warstwę.
Robbie Dee,
To nie jest złe, ale nadmierne uproszczenie. Przekonasz się, że w prawdziwym kodzie public List<Guid> ReturnEmployeeIds(bool includeManagement) { calculateSomethingHereForBothBranches; if (includeManagement) return something else return something else }podział na dwie metody prawdopodobnie oznacza, że ​​te dwie metody będą wymagały wyniku pierwszego obliczenia jako parametru.
Doc Brown
4
Myślę, że wszyscy mówimy, że publiczna twarz twojej klasy powinna prawdopodobnie ujawnić dwie metody (każda z jedną odpowiedzialnością), ale wewnętrznie każda z tych metod może rozsądnie przekazać żądanie do jednej prywatnej metody z rozgałęzionym parametrem bool .
MetaFight,
1
Mogę sobie wyobrazić 3 przypadki zachowania różniącego się między dwiema funkcjami. 1. Istnieje wstępne przetwarzanie, które jest inne. Niech funkcje publiczne wstępnie przetwarzają argumenty na funkcję prywatną. 2. Istnieje inne przetwarzanie końcowe. Poproś funkcje publiczne o przetworzenie wyniku funkcji prywatnej. 3. Istnieje pośrednie zachowanie, które się różni. Można to przekazać jako funkcję lambda do funkcji prywatnej, zakładając, że Twój język ją obsługuje. Często prowadzi to do nowej abstrakcji wielokrotnego użytku, która wcześniej tak naprawdę nie przyszła ci do głowy.
jpmc26
1

Dla programistów, którzy nie znają się na systemie, trudno byłoby zgadnąć, co zrobiła prawda. Najpierw umieść wskaźnik myszy nad nazwą metody lub przejdź do definicji (z których żadne nie jest dużym zadaniem)

Tak, prawda. Kod samodokumentujący to piękna rzecz. Jak wskazały inne odpowiedzi, istnieją sposoby na usunięcie dwuznaczności wywołania ReturnEmployeeIdsza pomocą wyliczenia, różnych nazw metod itp. Jednak czasami tak naprawdę nie można uniknąć tej sprawy, ale nie chcesz odejść i użyć je wszędzie (chyba że podoba ci się gadatliwość wizualna).

Na przykład może pomóc wyjaśnić jedno połączenie, ale niekoniecznie drugie.

Argument nazwany może być pomocny tutaj:

var ids = ReturnEmployeeIds(includeManagement: true);

Nie dodawaj dużo dodatkowej jasności (zgaduję, że to jest analizowanie wyliczenia):

Enum.Parse(typeof(StringComparison), "Ordinal", ignoreCase: true);

Może faktycznie zmniejszyć przejrzystość (jeśli dana osoba nie rozumie, jaka jest pojemność na liście):

var employeeIds = new List<int>(capacity: 24);

Lub gdzie to nie ma znaczenia, ponieważ używasz dobrych nazw zmiennych:

bool includeManagement = true;
var ids = ReturnEmployeeIds(includeManagement);

Czy jest gdzieś, gdzie formalnie dyskutuje się, czy jawnie określić parametry opcjonalne, gdy kompilator nie wymaga tego?

Nie AFAIK. Zajmijmy się jednak obiema częściami: jedynym jawnym zastosowaniem parametrów nazwanych jest to, że wymaga tego kompilator (zazwyczaj jest to usunięcie niejednoznaczności instrukcji). Poza tym możesz z nich korzystać, kiedy tylko chcesz. W tym artykule od MS znajduje się kilka sugestii, kiedy możesz z nich skorzystać, ale nie jest to szczególnie ostateczne https://msdn.microsoft.com/en-us/library/dd264739.aspx .

Osobiście uważam, że używam ich najczęściej podczas tworzenia niestandardowych atrybutów lub usuwania nowej metody, w której moje parametry mogą się zmieniać lub zmieniać kolejność (atrybuty nazwane b / c mogą być wymienione w dowolnej kolejności). Dodatkowo używam ich tylko wtedy, gdy podaję wartość statyczną inline, w przeciwnym razie jeśli przekazuję zmienną, staram się używać dobrych nazw zmiennych.

TL; DR

Ostatecznie sprowadza się to do osobistych preferencji. Oczywiście istnieje kilka przypadków użycia, w których musisz użyć nazwanych parametrów, ale potem musisz wykonać wywołanie osądu. IMO - używaj go w dowolnym miejscu, które Twoim zdaniem pomoże udokumentować kod, zmniejszy dwuznaczność lub pozwoli chronić podpisy metod.


źródło
0

Jeżeli parametr jest oczywiste ze względu na (w pełni kwalifikowana) Nazwa metody i to istnienie jest naturalną metodą, co ma robić, należy nie używać nazwie składni parametrów. Na przykład, ReturnEmployeeName(57)to jest cholernie oczywiste, że 57jest to identyfikator pracownika, więc zbędne jest dodawanie adnotacji za pomocą składni o nazwie parametru.

Z ReturnEmployeeIds(true), tak jak powiedziałeś, chyba że spojrzysz na deklarację / dokumentację funkcji, nie można ustalić, co to trueznaczy - więc powinieneś użyć nazwanej składni parametru.

Teraz rozważ ten trzeci przypadek:

void PrintEmployeeNames(bool includeManagement) {
    foreach (id; ReturnEmployeeIds(includeManagement)) {
        Console.WriteLine(ReturnEmployeeName(id));
    }
}

Składnia nazwanego parametru nie jest tutaj używana, ale ponieważ przekazujemy zmienną do ReturnEmployeeIds, a ta zmienna ma nazwę, a nazwa ta oznacza to samo, co parametr, do którego ją przekazujemy (często tak jest - ale nie zawsze!) - nie potrzebujemy składni nazwanego parametru, aby zrozumieć jego znaczenie z kodu.

Zasada jest więc prosta - jeśli z samego wywołania metody można łatwo zrozumieć, co ten parametr ma znaczyć, nie używaj nazwanego parametru. Jeśli nie możesz - to jest brak czytelności kodu i prawdopodobnie chcesz je naprawić (oczywiście nie za wszelką cenę, ale zwykle chcesz, aby kod był czytelny). Poprawka nie musi nazywać się parametrami - możesz najpierw umieścić wartość w zmiennej lub użyć metod sugerowanych w innych odpowiedziach, o ile wynik końcowy ułatwia zrozumienie działania parametru.

Idan Arye
źródło
7
Nie zgadzam się, ReturnEmployeeName(57)że natychmiast staje się oczywiste, że 57to identyfikator. GetEmployeeById(57)z drugiej strony ...
Hugo Zink
@ HugoZink Początkowo chciałem użyć czegoś takiego w tym przykładzie, ale celowo zmieniłem to, aby ReturnEmployeeNamezademonstrować przypadki, w których nawet bez wyraźnego podania parametru w nazwie metody, całkiem rozsądne jest oczekiwanie, że ludzie zrozumieją, co to jest. W każdym razie warto zauważyć, że w przeciwieństwie do tego ReturnEmployeeName, nie można rozsądnie zmienić nazwy ReturnEmployeeIdsna coś, co wskazuje na znaczenie parametrów.
Idan Arye,
1
W każdym razie jest mało prawdopodobne, abyśmy napisali ReturnEmployeeName (57) w prawdziwym programie. O wiele bardziej prawdopodobne jest, że napisalibyśmy ReturnEmployeeName (workerId). Dlaczego mielibyśmy kodować identyfikator pracownika? Chyba że jest to identyfikator programisty i dzieje się coś w rodzaju oszustwa, na przykład dodać trochę do wypłaty tego pracownika. :-)
Jay
0

Tak, myślę, że to dobry styl.

Ma to sens w przypadku stałych logicznych i stałych całkowitych.

Jeśli podajesz zmienną, nazwa zmiennej powinna wyjaśnić znaczenie. Jeśli nie, powinieneś nadać zmiennej lepszą nazwę.

Jeśli przekazujesz ciąg, znaczenie jest często jasne. Na przykład, jeśli widzę wywołanie GetFooData („Oregon”), myślę, że czytelnik może zgadnąć, że parametr jest nazwą stanu.

Oczywiście nie wszystkie łańcuchy są oczywiste. Jeśli widzę GetFooData („M”), to jeśli nie znam funkcji, nie mam pojęcia, co oznacza „M”. Jeśli jest to jakiś kod, na przykład M = „pracownicy na poziomie zarządzania”, prawdopodobnie powinniśmy mieć wyliczenie zamiast literału, a nazwa wyliczenia powinna być jasna.

I przypuszczam, że sznurek może wprowadzać w błąd. Może „Oregon” w powyższym przykładzie odnosi się nie do stanu, ale do produktu, klienta lub czegokolwiek innego.

Ale GetFooData (prawda) lub GetFooData (42) ... to może znaczyć wszystko. Nazwane parametry to wspaniały sposób na samodokumentowanie. Używałem ich do tego celu przy wielu okazjach.

Sójka
źródło
0

Nie ma żadnych super jasnych powodów, dla których warto używać tego typu składni.

Ogólnie staraj się unikać argumentów logicznych, które z daleka mogą wyglądać na arbitralne. includeManagement(najprawdopodobniej) znacznie wpłynie na wynik. Ale argument wygląda na to, że ma „małą wagę”.

Wykorzystanie wyliczenia zostało omówione, nie tylko będzie wyglądało, że argument „ma większy ciężar”, ale zapewni także skalowalność metody. Może to jednak nie być najlepsze rozwiązanie we wszystkich przypadkach, ponieważ twoja ReturnEmployeeIdsmetoda będzie musiała skalować się wraz z WhatToInclude-enum (patrz odpowiedź NiklasJ ). Może to powodować ból głowy później.

Zastanów się: jeśli WhatToIncludeskalujesz -enum, ale nie -metod ReturnEmployeeIds. Może wtedy wyrzucić ArgumentOutOfRangeException(najlepszy przypadek) lub zwrócić coś całkowicie niechcianego ( nulllub pustego List<Guid>). Co w niektórych przypadkach może mylić programistę, szczególnie jeśli ReturnEmployeeIdsznajdujesz się w bibliotece klas, w której kod źródłowy nie jest łatwo dostępny.

Można założyć, że WhatToInclude.Traineeszadziała, jeśli WhatToInclude.Alltak, ponieważ stażyści są „podzbiorem” wszystkich.

Zależy to (oczywiście) od sposobu ReturnEmployeeIds implementacji.


W przypadkach, w których można przekazać argument boolowski, staram się zamiast tego podzielić go na dwie metody (lub więcej, jeśli to konieczne), w twoim przypadku; można by wyodrębnić ReturnAllEmployeeIds, ReturnManagementIdsa ReturnRegularEmployeeIds. Obejmuje to wszystkie podstawy i jest całkowicie zrozumiałe dla każdego, kto je wdraża. Nie będzie to również miało wspomnianego wyżej problemu „skalowania”.

Ponieważ są tylko dwa wyniki dla czegoś, co ma logiczny argument. Wdrożenie dwóch metod wymaga bardzo niewiele dodatkowego wysiłku.

Mniej kodu, rzadko lepiej.


Z tym powiedział, nie pewne przypadki, w których wyraźnie deklarując argument poprawia czytelność. Zastanów się na przykład. GetLatestNews(max: 10). GetLatestNews(10)jest wciąż dość oczywiste, wyraźna deklaracja maxpomoże wyjaśnić wszelkie nieporozumienia.

A jeśli absolutnie musisz mieć argument boolowski, którego użycia nie można wywnioskować po prostu czytając truelub false. Powiedziałbym wtedy, że:

var ids = ReturnEmployeeIds(includeManagement: true);

.. jest absolutnie lepszy i bardziej czytelny niż:

var ids = ReturnEmployeeIds(true);

Ponieważ w drugim przykładzie truemoże to oznaczać absolutnie wszystko . Ale starałbym się uniknąć tego argumentu.

umrzeć maus
źródło