W projekcie znalazłem taki kod:
class SomeClass
{
private SomeType _someField;
public SomeType SomeField
{
get { return _someField; }
set { _someField = value; }
}
protected virtual void SomeMethod(/*...., */SomeType someVar)
{
}
private void SomeAnotherMethod()
{
//.............
SomeMethod(_someField);
//.............
}
};
Jak przekonać kolegów z drużyny, że to zły kod?
Uważam, że jest to niepotrzebna komplikacja. Po co przekazywać zmienną składową jako parametr metody, jeśli masz już do niej dostęp? Jest to również naruszenie enkapsulacji.
Czy widzisz jakieś inne problemy z tym kodem?
Odpowiedzi:
Myślę, że jest to ważny temat, ale otrzymujesz mieszane odpowiedzi ze względu na sposób, w jaki powstało pytanie. Osobiście miałem te same doświadczenia ze swoim zespołem, w których przekazywanie członków jako argumentów było niepotrzebne i zawijało kod. Mielibyśmy klasę, która działa z zestawem elementów, ale niektóre funkcje mają bezpośredni dostęp do elementów, a inne funkcje modyfikują te same elementy za pomocą parametrów (tj. Używają zupełnie innych nazw) i nie ma absolutnie żadnego technicznego powodu. Z technicznego punktu widzenia mam na myśli przykład podany przez Kate.
Radziłbym cofnąć się o krok i zamiast skupiać się na przekazywaniu członków jako parametrach, zainicjuj dyskusje ze swoim zespołem na temat przejrzystości i czytelności. Albo bardziej formalnie, albo tylko na korytarzach, przedyskutuj, co sprawia, że niektóre segmenty kodu są łatwiejsze do odczytania, a inne segmenty kodu trudniejsze. Następnie określ miary jakości lub atrybuty czystego kodu, o które jako zespół chciałbyś dążyć. W końcu, nawet podczas pracy nad projektami typu green field, spędzamy ponad 90% czasu na czytaniu i jak tylko kod zostanie napisany (powiedzmy 10-15 minut później), przechodzi on do konserwacji, gdzie czytelność jest jeszcze ważniejsza.
Tak więc w twoim konkretnym przykładzie użyłbym argumentu, że mniej kodu jest zawsze łatwiejsze do odczytania niż więcej kodu. Funkcja, która ma 3 parametry, jest trudniejsza do przetworzenia dla mózgu niż funkcja, która nie ma żadnego lub jednego parametru. Jeśli istnieje inna nazwa zmiennej, mózg musi śledzić jeszcze jedną rzecz podczas czytania kodu. Więc pamiętajmy „int m_value”, a następnie „int localValue” i pamiętajmy, że jeden naprawdę oznacza, że drugi jest zawsze droższy dla mózgu niż praca z „m_value”.
Aby uzyskać więcej amunicji i pomysłów, polecam zabranie kopii Czystego Kodu Wujka Boba .
źródło
Mogę myśleć o uzasadnieniu przekazania pola członka jako parametru w metodzie (prywatnej): wyraźnie określa, od czego zależy twoja metoda.
Jak mówisz, wszystkie pola składowe są niejawnymi parametrami twojej metody, ponieważ jest to cały obiekt. Czy jednak pełny obiekt jest naprawdę potrzebny do obliczenia wyniku? Jeśli
SomeMethod
jest to metoda wewnętrzna, która zależy tylko od tego_someField
, czy nie jest bardziej przejrzyste, aby ta zależność była wyraźna? W rzeczywistości wyraźne zdefiniowanie tej zależności może również sugerować, że można zrefaktoryzować ten fragment kodu poza klasę! (Uwaga: zakładam, że nie mówimy tutaj o programach pobierających ani ustawiających, ale kod, który faktycznie coś oblicza)Nie podałbym tego samego argumentu dla metod publicznych, ponieważ program wywołujący nie wie ani nie dba o to, która część obiektu jest istotna do obliczenia wyniku ...
źródło
_someField
jest to jedyny parametr potrzebny do obliczenia wyniku, i właśnie wyraziliśmy to jasno. (. Uwaga: to jest ważne Nie dodawać zależność, że staje się on wyraźny!)this
(lubself
), ale jest to wyraźnie zaznaczone przez samo wywołanieobj.method(x)
). Inne niejawne zależności to stan obiektu; zwykle utrudnia to zrozumienie kodu. Gdy tylko jest to możliwe - i w granicach rozsądku - wyraźne zależności i styl funkcjonalny. W przypadku metod prywatnych, jeśli to możliwe, jawnie przekaż każdy potrzebny parametr. I tak, pomaga je refaktoryzować.Widzę jeden silny powód, aby przekazywać zmienne składowe jako argumenty funkcji do metod prywatnych - czystość funkcji. Zmienne składowe są faktycznie stanem globalnym z punktu widzenia, a ponadto zmiennym stanem globalnym, jeśli wymieniony element zmienia się podczas wykonywania metody. Zastępując referencje zmiennych składowych parametrami metody, możemy skutecznie uczynić funkcję czystą. Czyste funkcje nie zależą od stanu zewnętrznego i nie powodują żadnych skutków ubocznych, zawsze zwracają te same wyniki, biorąc pod uwagę ten sam zestaw parametrów wejściowych - co ułatwia testowanie i konserwację w przyszłości.
Oczywiście nie jest łatwo ani praktyczne, aby wszystkie metody były czystymi metodami w języku OOP. Sądzę jednak, że zyskujesz znacznie na przejrzystości kodu dzięki metodom, które obsługują złożoną logikę czystą i używają niezmiennych zmiennych, przy jednoczesnym zachowaniu nieczystej „globalnej” obsługi stanu oddzielonej w ramach dedykowanych metod.
Przekazywanie zmiennych składowych do funkcji publicznej tego samego obiektu podczas wywoływania funkcji na zewnątrz stanowiłoby jednak moim zdaniem poważny zapach kodu.
źródło
Jeśli funkcja jest wywoływana w różnych momentach, czasami przekazując tę zmienną składową, a czasami przekazując coś innego, wtedy jest OK. Na przykład wcale nie uważałbym tego za złe:
gdzie
newStartDate
jest jakaś zmienna lokalna im_StartDate
jest zmienną składową.Jeśli jednak funkcja zostanie wywołana tylko wtedy, gdy zmienna członkowska zostanie do niej przekazana, to dziwne. Funkcje składowe działają przez cały czas na zmiennych składowych. Być może robią to (w zależności od języka, w którym pracujesz), aby uzyskać kopię zmiennej składowej - jeśli tak jest i nie możesz tego zobaczyć, kod może być ładniejszy, jeśli wyrazi cały proces.
źródło
Nikt się nie dotknął, że SomeMethod jest chroniony wirtualnie. Oznacza to, że klasa pochodna może z niej korzystać ORAZ ponownie wdrożyć swoją funkcjonalność. Klasa pochodna nie miałaby dostępu do zmiennej prywatnej, a zatem nie mogłaby zapewnić niestandardowej implementacji SomeMethod zależnej od zmiennej prywatnej. Zamiast przejmować zależność od zmiennej prywatnej, deklaracja wymaga od osoby dzwoniącej przekazania jej.
źródło
Frameworki GUI zazwyczaj mają jakąś klasę „Widok”, która reprezentuje rzeczy narysowane na ekranie, i ta klasa zazwyczaj zapewnia metodę na przykład
invalidateRect(Rect r)
zaznaczenia części obszaru rysowania jako wymagającej przerysowania. Klienci mogą wywołać tę metodę, aby zażądać aktualizacji części widoku. Ale widok może również wywoływać własną metodę, na przykład:spowodować przerysowanie całego obszaru. Na przykład może to zrobić, gdy zostanie po raz pierwszy dodany do hierarchii widoku.
Nie ma w tym nic złego - ramka widoku jest poprawnym prostokątem, a sam widok wie, że chce się narysować. Klasa View może udostępnić osobną metodę, która nie przyjmuje parametrów i używa zamiast tego ramki widoku:
Ale po co dodawać do tego specjalną metodę, skoro można użyć bardziej ogólnej
invalidateRect()
? Lub, jeśli zdecydujesz się na dostarczenieinvalidateFrame()
, najprawdopodobniej zaimplementujesz go w kategoriach bardziej ogólnychinvalidateRect()
:Państwo powinno przekazać zmienne instancji jako parametry do własnej metody , jeśli metoda nie działa w szczególności na tej zmiennej instancji. W powyższym przykładzie ramka widoku jest tylko kolejnym prostokątem, jeśli chodzi o
invalidateRect()
metodę.źródło
Ma to sens, jeśli metoda jest metodą użytkową. Powiedz na przykład, że musisz uzyskać unikalną krótką nazwę z kilku ciągów dowolnego tekstu.
Nie chcesz kodować osobnej implementacji dla każdego łańcucha, zamiast tego przekazanie łańcucha do wspólnej metody ma sens.
Jednak jeśli metoda zawsze działa na pojedynczym elemencie, wydaje się trochę głupim przekazywanie jej jako parametru.
źródło
Głównym powodem posiadania zmiennej członka w klasie jest umożliwienie jej ustawienia w jednym miejscu i udostępnienia jej wartości każdej innej metodzie w klasie. Ogólnie rzecz biorąc, można oczekiwać, że nie ma potrzeby przekazywania zmiennej składowej do metody klasy.
Mogę jednak wymyślić kilka powodów, dla których możesz chcieć przekazać zmienną składową do innej metody klasy. Po pierwsze, jeśli musisz zagwarantować, że wartość zmiennej składowej musi zostać użyta w niezmienionej formie, gdy jest używana z wywoływaną metodą, nawet jeśli ta metoda będzie musiała zmienić rzeczywistą wartość zmiennej składowej w pewnym momencie procesu. Drugi powód jest związany z pierwszym, ponieważ możesz chcieć zagwarantować niezmienność wartości w ramach łańcucha metod - na przykład podczas implementacji płynnej składni.
Mając to wszystko na uwadze, nie posunąłbym się nawet do stwierdzenia, że kod jest „zły” jako taki, jeśli przekażesz zmienną składową do jednej z metod klasy. Sugerowałbym jednak, że generalnie nie jest idealny, ponieważ może zachęcać do dużego duplikowania kodu, gdy parametr jest przypisany na przykład do zmiennej lokalnej, a dodatkowe parametry dodają „szum” w kodzie, gdzie nie jest potrzebny. Jeśli jesteś fanem książki Clean Code, wiesz, że wspomina ona o tym, że należy ograniczyć liczbę parametrów metody do jak najmniejszego minimum i tylko wtedy, gdy nie ma innego bardziej sensownego sposobu na uzyskanie dostępu do parametru .
źródło
Kilka rzeczy, które przychodzą mi do głowy, gdy o tym myślę:
źródło
Brakuje, jeśli parametr („argument”) jest referencyjny lub tylko do odczytu.
Wielu programistów zwykle przypisuje bezpośrednio wewnętrzne pola zmiennej w metodach konstruktora. Jest kilka wyjątków.
Pamiętaj, że „ustawiacze” mogą mieć dodatkowe metody, nie tylko przypisanie, a czasem nawet metody wirtualne lub wywoływać metody wirtualne.
Uwaga: Proponuję zachować wewnętrzne pola właściwości jako „chronione”.
źródło