Sprawdzanie poprawności parametrów konstruktora w C # - Najlepsze praktyki

34

Jaka jest najlepsza praktyka do sprawdzania poprawności parametrów konstruktora?

Załóżmy prosty C #:

public class MyClass
{
    public MyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            throw new ArgumentException("Text cannot be empty");

        // continue with normal construction
    }
}

Czy można zgodzić się na wyjątek?

Alternatywą, którą napotkałem, była wstępna weryfikacja, przed utworzeniem wystąpienia:

public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
        {
            MessageBox.Show("Text cannot be empty");
            return null;
        }
        else
        {
            return new MyClass(text);
        }
    }
}
MPelletier
źródło
10
„czy można rzucić wyjątek?” Jaka jest alternatywa?
S.Lott,
10
@ S.Lott: Nic nie rób. Ustaw wartość domyślną. Wydrukuj mesasge na pliku konsoli / logu. Zadzwoń Mrugnij światłem.
FrustratedWithFormsDesigner
3
@FrustratedWithFormsDesigner: „Nic nie rób?” W jaki sposób spełnione zostanie ograniczenie „pusty tekst”? „Ustaw wartość domyślną”? Jak zostanie użyta wartość argumentu tekstowego? Inne pomysły są w porządku, ale te dwa prowadziłyby do obiektu w stanie, który narusza ograniczenia tej klasy.
S.Lott,
1
@ S.Lott W obawie przed powtórzeniem się, byłem otwarty na inne podejście, którego nie znałem. Wierzę, że nie wszyscy wiem, tyle wiem na pewno.
MPelletier
3
@MPelletier, sprawdzanie parametrów z wyprzedzeniem spowoduje naruszenie DRY. (Nie powtarzaj się) Ponieważ musisz skopiować tę weryfikację wstępną do dowolnego kodu, który próbuje utworzyć instancję tej klasy.
CaffGeek

Odpowiedzi:

26

Mam tendencję do wykonywania całej mojej weryfikacji w konstruktorze. Jest to konieczne, ponieważ prawie zawsze tworzę niezmienne obiekty. W twoim konkretnym przypadku myślę, że jest to do przyjęcia.

if (string.IsNullOrEmpty(text))
    throw new ArgumentException("message", "text");

Jeśli używasz platformy .NET 4, możesz to zrobić. Oczywiście zależy to od tego, czy uważasz, że łańcuch zawierający tylko białe znaki jest nieprawidłowy.

if (string.IsNullOrWhiteSpace(text))
    throw new ArgumentException("message", "text");
ChaosPandion
źródło
Nie jestem pewien, czy jego „konkretny przypadek” jest wystarczająco konkretny, aby udzielić tak konkretnej porady. Nie mamy pojęcia, czy w tym kontekście sensowne jest zgłoszenie wyjątku lub po prostu zezwolenie na istnienie obiektu.
FrustratedWithFormsDesigner
@FrustratedWithFormsDesigner - Zakładam, że głosowałeś na mnie za to? Oczywiście masz rację, że dokonuję ekstrapolacji, ale oczywiście ten teoretyczny obiekt musi być nieprawidłowy, jeśli tekst wejściowy jest pusty. Czy naprawdę chcesz zadzwonić, Initaby otrzymać kod błędu, gdy wyjątek będzie działał równie dobrze i zmusić obiekt do utrzymywania prawidłowego stanu?
ChaosPandion
2
Mam tendencję do dzielenia tego pierwszego przypadku na dwa osobne wyjątki: jeden, który sprawdza nieważność i wyrzuca ArgumentNullExceptiondrugi, i drugi, który sprawdza pusty i wyrzuca an ArgumentException. Pozwala dzwoniącemu być bardziej wybrednym, jeśli chce w obsłudze wyjątków.
Jesse C. Slicer
1
@Jesse - Użyłem tej techniki, ale wciąż jestem na przeszkodzie co do jej ogólnej przydatności.
ChaosPandion
3
+1 za niezmienne przedmioty! Używam ich również tam, gdzie to możliwe (osobiście prawie zawsze znajduję).
22

Wiele osób twierdzi, że konstruktorzy nie powinni wprowadzać wyjątków. Na przykład KyleG na tej stronie właśnie to robi. Szczerze mówiąc, nie mogę wymyślić powodu, dla którego nie.

W C ++ rzucanie wyjątku od konstruktora jest złym pomysłem, ponieważ pozostawia ci przydzieloną pamięć zawierającą niezainicjowany obiekt, do którego nie masz odniesienia (tj. Jest to klasyczny wyciek pamięci). Prawdopodobnie stąd bierze się piętno - grupa oldschoolowych programistów C ++ na wpół rozwinęła naukę języka C # i po prostu zastosowała do tego to, co znali z C ++. Natomiast w Objective-C Apple oddzielił krok alokacji od kroku inicjalizacji, więc konstruktorzy w tym języku mogą zgłaszać wyjątki.

C # nie może wyciec pamięci z nieudanego wywołania konstruktora. Nawet niektóre klasy w środowisku .NET będą zgłaszać wyjątki w swoich konstruktorach.

Mrówka
źródło
Masz zamiar potargać piórami swoim komentarzem do C ++. :)
ChaosPandion
5
Ehh, C ++ jest świetnym językiem, ale jednym z moich ulubionych pomysłów są ludzie, którzy dobrze uczą się jednego języka, a następnie cały czas tak piszą . C # nie jest C ++.
Ant
10
Nadal może być niebezpieczne generowanie wyjątków od ctor w C #, ponieważ ctor mógł przydzielić niezarządzane zasoby, które nigdy nie zostaną usunięte. Finalizator musi być napisany, aby był defensywny wobec scenariusza, w którym finalizowany jest niekompletny obiekt. Ale te scenariusze są stosunkowo rzadkie. Nadchodzący artykuł na temat Centrum deweloperów C # będzie omawiać te scenariusze i sposoby ich defensywnego kodowania, więc uważaj na to miejsce, aby uzyskać szczegółowe informacje.
Eric Lippert
6
co? wyrzucenie wyjątku od ctor jest jedyną rzeczą, którą możesz zrobić w c ++, jeśli nie możesz skonstruować obiektu, i nie ma powodu , aby to spowodowało wyciek pamięci (choć oczywiście tak jest).
jk.
@jk: Hmm, masz rację! Choć wydaje się, że alternatywą jest oflagowanie złych obiektów jako „zombie”, co STL najwyraźniej robi zamiast rzucać wyjątki: yosefk.com/c++fqa/exceptions.html#fqa-17.2 Rozwiązanie Objective-C jest znacznie bardziej uporządkowane.
Ant
12

Zgłoszenie wyjątku IFF klasy nie można wprowadzić w spójny stan w odniesieniu do jej zastosowania semantycznego. W przeciwnym razie nie. NIGDY nie pozwól, aby obiekt istniał w niespójnym stanie. Obejmuje to niedostarczenie kompletnych konstruktorów (np. Posiadanie pustego konstruktora + inicjalizacja () przed faktycznym ukończeniem budowy obiektu) ... TYLKO POWIEDZ NIE!

W skrócie, wszyscy to robią. Zrobiłem to innego dnia dla bardzo wąsko używanego obiektu w wąskim zakresie. Pewnego dnia, ja lub ktoś inny prawdopodobnie zapłacę cenę za ten kupon w praktyce.

Powinienem zauważyć, że przez „konstruktor” rozumiem rzecz, którą klient wywołuje w celu zbudowania obiektu. Równie dobrze może to być coś innego niż faktyczna konstrukcja o nazwie „Konstruktor”. Na przykład coś takiego w C ++ nie naruszy zasady IMNSHO:

struct funky_object
{
  ...
private:
  funky_object();
  bool initialize(std::string);

  friend boost::optional<funky_object> build_funky(std::string);
};
boost::optional<funky_object> build_funky(std::string str)
{
  funky_object fo;
  if (fo.initialize(str)) return fo;
  return boost::optional<funky_object>();
}

Ponieważ jedynym sposobem na utworzenie a funky_objectjest wywołaniebuild_funky zasady, aby nigdy nie dopuszczać do istnienia niepoprawnego obiektu, pozostaje nienaruszony, nawet jeśli faktyczny „Konstruktor” nie zakończy zadania.

To jednak dużo dodatkowej pracy dla wątpliwego zysku (może nawet straty). Nadal wolałbym trasę wyjątkową.

Edward Strange
źródło
9

W takim przypadku użyłbym metody fabrycznej. Zasadniczo, ustaw swoją klasę tylko na prywatne konstruktory i metodę fabryczną, która zwraca instancję twojego obiektu. Jeśli początkowe parametry są niepoprawne, po prostu zwróć null i poproś kod wywołujący o podjęcie decyzji.

public class MyClass
{
    private MyClass(string text)
    {
        //normal construction
    }

    public static MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            return null;
        else
            return new MyClass(text);
    }
}
public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        var cls = MyClass.MakeMyClass(text);
        if(cls == null)
             //show messagebox or throw exception
        return cls;
    }
}

Nigdy nie rzucaj wyjątków, chyba że warunki są wyjątkowe. Myślę, że w tym przypadku pustą wartość można łatwo przekazać. W takim przypadku użycie tego wzorca pozwoliłoby uniknąć wyjątków i kar za wydajność, które ponoszą, przy jednoczesnym utrzymaniu prawidłowego stanu MyClass.

Donn Relacion
źródło
1
Nie widzę przewagi. Dlaczego testowanie wyniku fabryki pod kątem wartości NULL jest lepsze niż próba złapania gdzieś, która może uchwycić wyjątek od konstruktora? Twoje podejście wydaje się „ignorować wyjątki, wróć do jawnego sprawdzania wartości zwracanej przez metody pod kątem błędów”. Dawno temu przyjęliśmy, że wyjątki są na ogół lepsze niż konieczność jawnego testowania wartości zwracanej przez każdą metodę pod kątem błędów, więc twoja rada wydaje się podejrzana. Chciałbym zobaczyć uzasadnienie, dlaczego uważasz, że ogólna zasada nie ma tutaj zastosowania.
DW
nigdy nie zaakceptowaliśmy faktu, że wyjątki były lepsze od kodów zwrotnych, jeśli są zbyt często rzucane, mogą być ogromnym hitem wydajności. Jednak zgłoszenie wyjątku od konstruktora spowoduje przeciek pamięci, nawet w .NET, jeśli przydzielono coś, co nie jest zarządzane. Aby nadrobić ten przypadek, musisz wykonać wiele pracy w finalizatorze. W każdym razie fabryka jest tutaj dobrym pomysłem, a TBH powinien zgłosić wyjątek zamiast zwracać zero. Zakładając, że stworzysz tylko kilka „złych” klas, wtedy preferowane jest wykonanie rzutu fabrycznego.
gbjbaanb,
2
  • Konstruktor nie powinien mieć żadnych skutków ubocznych.
    • Coś więcej niż inicjalizacja pola prywatnego powinna być postrzegana jako efekt uboczny.
    • Konstruktor z efektami ubocznymi łamie zasadę pojedynczej odpowiedzialności (SRP) i działa wbrew duchowi programowania obiektowego (OOP).
  • Konstruktor powinien być lekki i nigdy nie powinien zawieść.
    • Na przykład zawsze wzdrygam się, gdy widzę blok try-catch w konstruktorze. Konstruktor nie powinien zgłaszać wyjątków ani rejestrować błędów.

Można rozsądnie zakwestionować te wytyczne i powiedzieć: „Ale nie przestrzegam tych zasad, a mój kod działa dobrze!” Na to odpowiedziałbym: „Cóż, to może być prawda, dopóki tak nie będzie”.

  • Wyjątki i błędy wewnątrz konstruktora są bardzo nieoczekiwane. O ile nie zostanie im to nakazane, przyszli programiści nie będą skłonni otaczać tych wywołań konstruktora kodem obronnym.
  • Jeśli coś zawiedzie w produkcji, wygenerowany ślad stosu może być trudny do przeanalizowania. Początek śledzenia stosu może wskazywać na wywołanie konstruktora, ale wiele rzeczy dzieje się w konstruktorze i może nie wskazywać na faktyczne LOC, które nie powiodło się.
    • Przetwarzałem wiele śladów stosu .NET tam, gdzie tak było.
Jim G.
źródło
0

To zależy od tego, co robi MyClass. Jeśli MyClass była w rzeczywistości klasą repozytorium danych, a tekst parametru był łańcuchem połączenia, najlepszą praktyką byłoby zgłoszenie wyjątku ArgumentException. Jeśli jednak MyClass jest klasą StringBuilder (na przykład), możesz po prostu pozostawić ją pustą.

Zależy to zatem od tego, jak istotny jest tekst parametru dla metody - czy obiekt ma sens z pustą lub pustą wartością?

Steven Striga
źródło
2
Myślę, że pytanie sugeruje, że klasa faktycznie wymaga, aby ciąg nie był pusty. Tak nie jest w przykładzie StringBuilder.
Sergio Acosta
Zatem odpowiedź musi brzmieć „tak” - dopuszczalny jest wyjątek. Aby uniknąć konieczności wychwytywania tego błędu, możesz użyć wzorca fabrycznego do obsługi tworzenia obiektu, który zawiera pustą wielkość ciągu.
Steven Striga
0

Preferuję ustawienie domyślne, ale wiem, że Java ma bibliotekę „Apache Commons”, która robi coś takiego, i myślę, że to również całkiem dobry pomysł. Nie widzę problemu z zgłaszaniem wyjątku, jeśli niepoprawna wartość spowodowałaby, że obiekt byłby niezdatny do użytku; ciąg nie jest dobrym przykładem, ale co, jeśli byłby to dla DI biednego człowieka? Nie można było działać, jeśli nullzamiast ICustomerRepositoryinterfejsu powiedzmy wartość . Powiedziałbym, że w takiej sytuacji zgłoszenie wyjątku jest właściwym sposobem postępowania.

Wayne Molina
źródło
-1

Kiedy pracowałem w c ++, nie używałem do rzucania wyjątków w konstruktorze, ponieważ może to prowadzić do wycieku pamięci, nauczyłem się tego na własnej skórze. Każdy, kto pracuje z c ++, wie, jak trudny i problematyczny może być wyciek pamięci.

Ale jeśli jesteś w języku C # / Java, nie masz tego problemu, ponieważ śmieciarz zbierze pamięć. Jeśli używasz języka C #, myślę, że lepiej jest używać właściwości w przyjemny i spójny sposób, aby upewnić się, że ograniczenia danych są gwarantowane.

public class MyClass
{
    private string _text;
    public string Text 
    {
        get
        {
            return _text;
        } 
        set
        {
            if (string.IsNullOrWhiteSpace(value))
                throw new ArgumentException("Text cannot be empty");
            _text = value;
        } 
    }

    public MyClass(string text)
    {
        Text = text;
        // continue with normal construction
    }
}
ajitdh
źródło
-3

Zgłoszenie wyjątku będzie prawdziwym bólem dla użytkowników twojej klasy. Jeśli sprawdzanie poprawności stanowi problem, generalnie tworzę obiekt jako proces dwuetapowy.

1 - Utwórz instancję

2 - Wywołaj metodę Initialize z wynikiem zwrotu.

LUB

Wywołaj metodę / funkcję, która utworzy dla ciebie klasę, która może przeprowadzić walidację.

LUB

Miej flagę isValid, co mi się nie podoba, ale niektórzy to robią.

Maczać
źródło
3
@Dunk, to po prostu źle.
CaffGeek
4
@Chad, to jest twoja opinia, ale moja opinia nie jest błędna. To po prostu różni się od twojego. Uważam, że kod try-catch jest znacznie trudniejszy do odczytania i widziałem znacznie więcej błędów powstających, gdy ludzie używają try-catch do trywialnej obsługi błędów niż w przypadku tradycyjnych metod. Takie było moje doświadczenie i nie jest źle. Jest jak jest.
Dunk
5
PUNKT polega na tym, że po wywołaniu konstruktora, jeśli nie tworzy, ponieważ dane wejściowe są nieprawidłowe, jest to WYJĄTEK. Dane aplikacji są teraz w niespójnym / nieprawidłowym stanie, jeśli po prostu utworzę obiekt i ustawię flagę isValid = false. Konieczność testowania po utworzeniu klasy, jeśli jest ona ważna, jest okropnym, bardzo podatnym na błędy projektem. Powiedziałeś, że masz konstruktora, a następnie zainicjuj wywołanie ... Co jeśli nie zainicjuję wywołania? Co wtedy? A jeśli inicjalizacja otrzyma złe dane, czy mogę teraz zgłosić wyjątek? Twoja klasa nie powinna być trudna w użyciu.
CaffGeek,
5
@Dunk, jeśli uważasz, że wyjątki są trudne w użyciu, używasz ich źle. Mówiąc najprościej, do konstruktora dostarczono złych danych wejściowych. To wyjątek. Aplikacja nie powinna kontynuować działania, chyba że zostanie naprawiona. Kropka. Jeśli tak, skończy się to gorszym problemem, złymi danymi. Jeśli nie wiesz, jak obsłużyć wyjątek, nie robisz tego, pozwalasz mu zwiększać stos wywołań, dopóki nie będzie można go obsłużyć, nawet jeśli oznacza to po prostu logowanie i zatrzymanie wykonania. Mówiąc wprost, nie musisz obsługiwać wyjątków ani ich testować. Po prostu działają.
CaffGeek
3
Niestety, jest to po prostu zbyt wiele anty-wzorca do rozważenia.
MPelletier