Mamy ten kod, który po uproszczeniu wygląda następująco:
public class Room
{
public Client Client { get; set; }
public long ClientId
{
get
{
return Client == null ? 0 : Client.Id;
}
}
}
public class Client
{
public long Id { get; set; }
}
Teraz mamy trzy punkty widzenia.
1) To jest dobry kod, ponieważ Client
właściwość powinna być zawsze ustawiona (tj. Nie pusta), więc Client == null
nigdy się nie pojawi, a wartość 0
Id i tak oznacza fałszywy identyfikator (taka jest opinia autora kodu ;-))
2) Nie można powoływać się na rozmówcy, aby wiedzieć, że 0
jest fałszywa wartość Id
, a gdy Client
nieruchomość powinna być zawsze należy rzucać exception
w get
gdy Client
nieruchomość zdarza się być null
3) Gdy Client
właściwość powinna być zawsze ustawiona, po prostu powracasz Client.Id
i pozwalasz kodowi zgłosić NullRef
wyjątek, gdy Client
właściwość jest zerowa.
Który z nich jest najbardziej poprawny? Czy jest czwarta możliwość?
c#
code-quality
null
Michel
źródło
źródło
Odpowiedzi:
Pachnie, że powinieneś ograniczyć liczbę stanów, w których
Room
może znajdować się klasa.Sam fakt, że pytasz o to, co zrobić, gdy
Client
jest pusty, jest wskazówką, żeRoom
przestrzeń stanu jest zbyt duża.Aby uprościć sprawę, nie pozwoliłbym, aby
Client
własność jakiejkolwiekRoom
instancji była zawsze pusta. Oznacza to, że kod wewnątrzRoom
może bezpiecznie założyć, żeClient
nigdy nie jest zerowy.Jeśli z jakiegoś powodu w przyszłości
Client
stanienull
oprzeć się pokusie, aby wspierać ten stan. Takie postępowanie zwiększy złożoność konserwacji.Zamiast tego pozwól, aby kod zawiódł i szybko zawiódł. W końcu nie jest to obsługiwany stan. Jeśli aplikacja przejdzie w ten stan, przekroczyłeś już linię bez powrotu. Jedyną rozsądną rzeczą do zrobienia jest zamknięcie aplikacji.
Może się to zdarzyć (całkiem naturalnie) w wyniku nieobsługiwanego wyjątku odwołania zerowego.
źródło
null
wartośćNullObject
(lub raczejNullClient
), która nadal działałaby z istniejącym kodem i pozwoliła zdefiniować zachowanie nieistniejącego klienta, jeśli zajdzie taka potrzeba. Pytanie brzmi, czy przypadek „braku klienta” jest częścią logiki biznesowej, czy nie.Kilka uwag:
a) Dlaczego istnieje getter specjalnie dla ClientId, skoro już istnieje publiczny getter dla samej instancji Client? Nie rozumiem, dlaczego informacja, że ClientId jest,
long
musi być wyryta w kamieniu w podpisie Pokoju.b) Jeśli chodzi o drugą opinię, możesz wprowadzić stałą
Invalid_Client_Id
.c) Jeśli chodzi o opinię pierwszą i trzecią (i co jest moim głównym punktem): pokój musi zawsze mieć klienta? Może to tylko semantyka, ale to nie brzmi dobrze. Być może bardziej odpowiednie byłoby posiadanie całkowicie oddzielnych klas dla Pokoju i Klienta oraz innej klasy, która je łączy. Może spotkanie, rezerwacja, obłożenie? (Zależy to od tego, co faktycznie robisz.) I na tej klasie możesz egzekwować ograniczenia „musi mieć pokój” i „musi mieć klienta”.
źródło
Nie zgadzam się ze wszystkimi trzema opiniami. Jeśli
Client
nigdy nie może byćnull
, to nawet nie pozwól, aby tak byłonull
!Client
konstruktoraArgumentNullException
do konstruktora.Więc twój kod byłby mniej więcej taki:
Nie ma to związku z twoim kodem, ale prawdopodobnie lepiej byłoby użyć niezmiennej wersji
Room
, robiąctheClient
readonly
, a następnie, jeśli klient się zmieni, tworząc nowy pokój. Zwiększy to bezpieczeństwo wątków w kodzie oprócz bezpieczeństwa zerowego innych aspektów mojej odpowiedzi. Zobacz tę dyskusję na temat zmiennych i niezmiennychźródło
ArgumentNullException
?NullReferenceException
, jest on generowany przez maszynę wirtualną .Net „, gdy następuje próba wyłuskiwania odwołania do obiektu zerowego” . Powinieneś rzucać anArgumentNullException
, który jest wyrzucany „, gdy odwołanie zerowe (Nic w Visual Basic) jest przekazywane do metody, która nie przyjmuje go jako prawidłowego argumentu”.readonly
). Właśnie edytowałbym, ale czułem, że komentarz mógłby lepiej wyjaśnić, dlaczego (dla przyszłych czytelników). Jeśli jeszcze nie udzieliłeś tej odpowiedzi, podałbym dokładnie to samo rozwiązanie. Niezmienność jest również dobrym pomysłem, ale nie zawsze jest to tak łatwe, jak się ma nadzieję.Należy unikać drugiej i trzeciej opcji - pobierający nie powinien uderzać dzwoniącego, z wyjątkiem tego, że nie ma nad nimi kontroli.
Powinieneś zdecydować, czy Klient może być kiedykolwiek zerowy. Jeśli tak, powinieneś umożliwić dzwoniącemu sprawdzenie, czy ma on wartość NULL, zanim uzyska do niego dostęp (np. Właściwość bool ClientIsNull).
Jeśli zdecydujesz, że Klient nigdy nie może mieć wartości NULL, ustaw go jako wymagany parametr dla konstruktora i wyślij tam wyjątek, jeśli zostanie przekazane NULL.
Wreszcie pierwsza opcja zawiera także zapach kodu. Powinieneś pozwolić Klientowi postępować z jego własną własnością ID. Wydaje się, że przesada koduje moduł pobierający w klasie kontenera, który po prostu wywołuje moduł pobierający w klasie zawartej. Po prostu ujawnij klienta jako właściwość (w przeciwnym razie skończysz duplikowaniem wszystkiego, co klient już oferuje ).
Jeśli Klient może mieć wartość zerową, wówczas przynajmniej ponosisz odpowiedzialność za dzwoniącego:
źródło
Jeśli właściwość null Client jest obsługiwanym stanem, rozważ użycie NullObject .
Ale najprawdopodobniej jest to wyjątkowy stan, więc powinieneś uniemożliwić (lub po prostu nie bardzo wygodne) skończyć na zero
Client
:Jeśli nie jest to obsługiwane, nie trać czasu na półpieczone roztwory. W tym przypadku:
„Rozwiązałeś” wyjątek NullReferenceEx tutaj, ale za wielką cenę! Zmusiłoby to wszystkich dzwoniących
Room.ClientId
do sprawdzenia 0:Dziwne błędy mogą powstać w przypadku innych programistów (lub ciebie!), Zapominając o sprawdzeniu wartości zwracanej „ErrorCode” w pewnym momencie.
Graj bezpiecznie i szybko się nie powiedzie. Zezwól na zgłoszenie wyjątku NullReferenceException i „z wdziękiem” złap go gdzieś wyżej na stosie wywołań, zamiast pozwolić dzwoniącemu na jeszcze większy bałagan…
Z drugiej strony, jeśli tak bardzo chcesz ujawnić,
Client
a takżeClientId
pomyśleć o TellDontAsk . Jeśli poprosisz zbyt wiele o przedmioty zamiast powiedzieć im, co chcesz osiągnąć, możesz zakończyć łączenie wszystkiego, co utrudni później zmianę.źródło
NotSupportedException
jest źle wykorzystywane, powinieneś użyćArgumentNullException
zamiast tego.Począwszy od wersji c # 6.0, która jest już wydana, powinieneś to zrobić,
Podnoszenie własności
Client
doRoom
jest oczywistym naruszeniem enkapsulacji i zasady DRY.Jeśli chcesz uzyskać dostęp do
Id
jednegoClient
zRoom
nich,Uwaga korzystanie z Null Operator warunkowy ,
clientId
będzielong?
, jeśliClient
jestnull
,clientId
będzienull
, w przeciwnym razieclientId
będzie miał wartośćClient.Id
.źródło
clientid
jest długi?var clientId = room.Client.Id
będzie zarówno bezpieczny, jak ilong
.