Jak programować? [zamknięte]

Cóż, to zależy, kto jest twoją publicznością.

Jeśli piszesz kod biblioteczny, którego spodziewasz się użyć przez wiele innych osób, które nie będą z tobą rozmawiać o tym, jak go używać, to nie jest to przesada. Docenią twój wysiłek.

(to powiedziawszy, jeśli to robisz, sugeruję zdefiniowanie lepszych WYJĄTKÓW niż tylko System.Wyjątek, aby ułatwić osobom, które chcą złapać niektóre z Twoich wyjątków, ale nie inne.)

Ale jeśli masz zamiar go użyć siebie (lub ciebie i twojego kumpla), to oczywiście przesada i pewnie w końcu cię boli, czyniąc Twój kod mniej czytelnym.

Chciałbym, żeby mój zespół tak kodował. Większość ludzi nawet nie rozumie sensu programowania defensywnego. Najlepsze, co robią, to zawinąć całą metodę w instrukcję try catch i pozwolić, aby wszystkie wyjątki były obsługiwane przez ogólny blok WYJĄTKÓW!

Czapki z głów Ian. Rozumiem twój dylemat. Sam przez to przeszedłem. Ale to, co zrobiłeś, prawdopodobnie pomogło jakiemuś deweloperowi kilka godzin uderzania klawiatury.

Pamiętaj, gdy używasz API. NET framework, czego się po nim spodziewasz? Co wydaje się naturalne? Zrób to samo ze swoim kodem.

Wiem, że to wymaga czasu. Ale wtedy jakość ma swoją cenę.

PS: naprawdę nie musisz obsługiwać wszystkich błędów i rzucać niestandardowego wyjątku. Pamiętaj, że twoja metoda będzie używana tylko przez innych programistów. Powinny one być w stanie samodzielnie ustalić wspólne wyjątki ramowe. To nie jest warte zachodu.

Twój przykład "przed" ma rozróżnienie: jest jasny i zwięzły.

Jeśli coś jest nie tak, wyjątek zostanie w końcu wyrzucony przez framework. Jeśli nie możesz nic zrobić z wyjątkiem, równie dobrze możesz pozwolić mu propagować stos wywołań.

Są jednak chwile, kiedy wyjątek jest wrzucany głęboko w ramy, które naprawdę nie rzuca światła na to, jaki jest rzeczywisty problem. Jeśli problem polega na tym, że nie masz poprawnego ciągu połączenia, ale framework rzuca wyjątek w stylu "invalid use of null", wtedy czasami lepiej jest złapać wyjątek i przerobić go z wiadomością, która jest bardziej znacząca.

Często sprawdzam obiekty null, ponieważ potrzebuję rzeczywistego obiektu do działania, a jeśli obiekt jest pusty, rzucony wyjątek będzie skośny. Ale sprawdzam obiekty null tylko wtedy, gdy Wiem, że to się stanie. Niektóre fabryki obiektów nie zwracają obiektów null; rzucają wyjątek zamiast tego sprawdzanie null będzie bezużyteczne w takich przypadkach.

Myślę, że nie napiszę żadnej z tych null-reference sprawdzania logiki-przynajmniej, nie sposób, w jaki to zrobiłeś.

Moje programy pobierające ustawienia konfiguracyjne z pliku konfiguracyjnego aplikacji sprawdzają wszystkie te ustawienia przy starcie. Zwykle buduję statyczną klasę, która zawiera ustawienia i odwołuje się do właściwości tej klasy (a nie ConfigurationManager) gdzie indziej w aplikacji. Są ku temu dwa powody.

Po pierwsze, jeśli aplikacja nie jest poprawnie skonfigurowana, to nie idzie do pracy. Wolałbym to wiedzieć w momencie, gdy program odczytuje plik konfiguracyjny, niż w pewnym momencie w przyszłości, gdy próbuję utworzyć połączenie z bazą danych.

Po Drugie, sprawdzenie poprawności konfiguracji nie powinno być problemem obiektów, które opierają się na konfiguracji. Nie ma sensu zmuszać się do wstawiania czeków w całym kodzie, jeśli już wykonałeś te czeki z góry. (Są od tego wyjątki, oczywiście-dla przykład, długotrwałe aplikacje, w których musisz być w stanie zmienić konfigurację podczas działania programu i mieć te zmiany odzwierciedlone w zachowaniu programu; w programach takich jak ten musisz przejść do ConfigurationManager, gdy potrzebujesz ustawienia.)

Ja też nie sprawdzałbym null-reference przy wywołaniach GetFactory i CreateConnection. Jak napisałbyś przypadek testowy, aby wykonać ten kod? Nie możesz, bo nie wiesz jak sprawić, by te metody zwracały null - nawet nie wiedz, że jest możliwe aby te metody zwróciły null. Więc zastąpiłeś jeden problem-twój program może rzucić NullReferenceException w Warunkach, których nie rozumiesz-innym, bardziej znaczącym: w tych samych tajemniczych warunkach, Twój program uruchomi kod, którego nie przetestowałeś.

Moja zasada kciuków to:

Don ' t catch if the message of the rzucony wyjątek ma znaczenie dla dzwoniący.

Tak więc, NullReferenceException nie ma odpowiedniej wiadomości, sprawdziłbym czy jest null i wrzucił wyjątek z lepszą wiadomością. ConfigurationErrorException ma znaczenie, więc go nie łapię.

Jedynym wyjątkiem jest sytuacja, gdy "kontrakt" GetConnection nie pobiera łańcucha połączeń w konfiguracji plik.

Jeśli jest tak, że GetConnection powinno mieć kontrakt z niestandardowym wyjątkiem, który mówi, że nie można odzyskać połączenia, możesz zawinąć ConfigurationErrorException w niestandardowym wyjątku.

Innym rozwiązaniem jest określenie, że GetConnection nie może rzucać (ale może zwracać null), a następnie dodajesz "ExceptionHandler" do swojej klasy.

Brakuje dokumentacji metody. ;-)

Każda metoda ma określone parametry wejściowe i wyjściowe oraz określone zachowanie wynikowe. W Twoim przypadku coś w stylu: "zwraca poprawne otwarte połączenie w przypadku powodzenia, else zwraca null(lub rzuca XXXException, jak chcesz). Pamiętając o tym zachowaniu, możesz teraz zdecydować, jak defensywnie powinieneś programować.

• Jeśli twoja metoda powinna ujawnić szczegółowe informacje, dlaczego i co się nie powiodło, zrób to tak, jak zrobiłeś i sprawdź i złap wszystko i wszystko i zwróć odpowiednie informacje.

• Ale, jeśli jesteś po prostu zainteresowany otwartą DBConnection lub po prostu null (lub jakiś wyjątek zdefiniowany przez użytkownika) w przypadku błędu, po prostu zawiń wszystko w try/catch i zwróć null (lub jakiś wyjątek) w przypadku błędu, a obiekt else.

Więc powiedziałbym, że zależy to od zachowania metody i oczekiwanego wyniku.

Ogólnie rzecz biorąc, wyjątki specyficzne dla bazy danych powinny być przechwytywane i ponownie wyrzucane jako coś bardziej ogólnego, jak (hipotetyczny) wyjątek DataAccessFailure. Kod wyższego poziomu w większości przypadków nie musi wiedzieć, że czytasz dane z bazy danych.

Innym powodem do szybkiego przechwytywania tych błędów jest to, że często zawierają one pewne szczegóły bazy danych w swoich wiadomościach, takie jak "No such table: ACCOUNTS_BLOCKED" lub "user key invalid: 234234". Jeśli to propaguje się do użytkownika końcowego, to jest złe w kilku sposoby:

1. mylące.
Potencjalne naruszenie bezpieczeństwa.
2. żenujące dla wizerunku Twojej firmy (wyobraź sobie, że klient czyta komunikat o błędzie z prymitywną gramatyką).

Zakodowałbym to dokładnie tak, jak przy pierwszej próbie.

Jednak użytkownik tej funkcji chroniłby obiekt połączenia za pomocą bloku USING.

Naprawdę nie lubię tłumaczyć WYJĄTKÓW, jak robią to twoje inne wersje, ponieważ bardzo trudno jest dowiedzieć się, dlaczego się zepsuł (baza danych w dół? Nie masz uprawnień do odczytu pliku konfiguracyjnego itp?).

Zmieniona wersja nie dodaje wiele wartości, o ile aplikacja ma funkcję obsługi AppDomain.UnexpectedException, która zrzuca łańcuch exception.InnerException i wszystkie ślady stosu do jakiegoś pliku dziennika (lub nawet lepiej, przechwytuje minidump), a następnie wywołuje Environment.FailFast.

Na podstawie tych informacji, w miarę łatwo będzie określić, co poszło nie tak, bez konieczności komplikowania kodu metody z dodatkowym sprawdzaniem błędów.

Zauważ, że lepiej jest obsługiwać AppDomain.UnexpectedException I wywoływać Environment.FailFast zamiast posiadanie najwyższego poziomu try/catch (Exception x), ponieważ w przypadku tego ostatniego, pierwotna przyczyna problemu prawdopodobnie zostanie przysłonięta przez dalsze wyjątki.

Dzieje się tak dlatego, że jeśli złapiesz wyjątek, wszystkie otwarte bloki finally zostaną uruchomione i prawdopodobnie wyrzucą więcej WYJĄTKÓW, co ukryje oryginalny wyjątek (lub co gorsza, usunie pliki w celu przywrócenia jakiegoś stanu, być może niewłaściwych plików, może nawet ważnych plików). Nigdy nie należy przechwytywać wyjątku wskazującego na nieprawidłową stan programu, z którym nie wiesz, jak sobie radzić - nawet w bloku najwyższego poziomu main funkcji try/catch. Obsługa AppDomain.UnexpectedException i wywołanie Environment.FailFast jest innym (i bardziej pożądanym) czajnikiem ryb, ponieważ zatrzymuje finally bloki przed uruchomieniem, a jeśli próbujesz zatrzymać swój program i zapisać przydatne informacje bez robienia dalszych szkód, zdecydowanie nie chcesz uruchamiać swoich {6]} bloków.

Nie zapomnij sprawdzić OutOfMemoryExceptions... wiesz, to może się zdarzyć.

Zmiany Iaina wyglądają dla mnie sensownie.

Jeśli używam systemu i używam go niewłaściwie, chcę jak najwięcej informacji o niewłaściwym użyciu. Np. jeśli zapomnę wstawić jakieś wartości do konfiguracji przed wywołaniem metody, chcę mieć InvalidOperationException z Komunikatem opisującym mój błąd, a nie Keynotfoundexception / NullReferenceException.

It ' s all about context IMO. Widziałem kilka dość nieprzeniknionych wiadomości WYJĄTKÓW w moim czasie, ale innym razem domyślne wyjątek pochodzący z frameworka jest w porządku.

Ogólnie rzecz biorąc, myślę, że lepiej jest błądzić po stronie ostrożności, zwłaszcza gdy piszesz coś, co jest mocno używane przez innych ludzi lub zazwyczaj głęboko w wykresie wywołania, gdzie błąd jest trudniejszy do zdiagnozowania.

Zawsze staram się pamiętać, że jako twórca kodu lub systemu jestem w lepszej sytuacji do diagnozowania błędów niż ktoś, kto właśnie z niego korzysta. Czasami zdarza się, że kilka linijek sprawdzanie kodu + niestandardowa wiadomość wyjątku może skumulować wiele godzin czasu debugowania (a także ułatwić sobie życie, ponieważ nie zostaniesz pociągnięty do cudzej maszyny, aby debugować swój problem).

W moich oczach twoja próbka " po " nie jest tak naprawdę defensywna. Ponieważ defensywne byłoby sprawdzenie części pod twoją kontrolą, które byłyby argumentem connectionName (sprawdź czy nie ma null lub empty i wyrzuć ArgumentNullException).

Dlaczego nie podzielić metody, którą masz po dodaniu całego programowania defensywnego? Masz kilka odrębnych bloków logicznych, które wymagają osobnych metod. Dlaczego? Ponieważ wtedy hermetyzujesz logikę, która należy do siebie, a wynikowa metoda Publiczna po prostu łączy te bloki we właściwy sposób.

Coś takiego (edytowane w edytorze SO, więc bez sprawdzania składni / kompilatora. Może się nie skompilować; -))

private string GetConnectionString(String connectionName)
{

   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
   if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
   return cs;
}

private DbProviderFactory GetFactory(String ProviderName)
{
   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
   return factory;
}

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs = GetConnectionString(connectionName);

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = GetFactory(cs.ProviderName);


   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

PS: To nie jest kompletny refaktor, tylko dwa pierwsze bloki.