Nieprawidłowa obsługa wyjątków może przynieść więcej problemów niż pożytku. O obsłudze błędów można byłoby napisać artykuł, jednak w poście chciałbym skupić się wyłącznie na kilku aspektach.
1. Pierwszym problemem jest fakt, że część programistów używa wyjątków do sytuacji po prostu niewyjątkowych. Jak sama nazwa mówi, wyjątek powinien być zastosowany gdzie może zdarzyć się coś niespodziewanego. Spójrzmy więc na poniższy fragment:
int number;
try
{
number = int.Parse(text);
}
catch
{
number = -1;
}
Załóżmy, że mamy aplikację w której użytkownik może wpisywać w pole jakiś tekst a następnie musimy to zamienić na liczbę. To, że użytkownik wpiszę tekst zamiast liczby nie jest wyjątkowe i stanowi normalny workflow. Powinniśmy zatem skorzystać z alternatywy np.:
int number;
if(int.TryParse(text, out number) == false)
number = -1;
Innym często spotykanym scenariuszem jest metoda przeszukująca bazę danych. Jeśli nie ma danego wiersza, nie powinniśmy wyrzucać wyjątków a po prostu NULL lub Null object pattern. W końcu jeśli implementujemy wyszukiwarkę, może się zdarzyć że szukana fraza nie istnieje w bazie danych:
Article FindArticle(string text)
{
Article article = ...
// jakis SELECT * FROM
if(article == null)
throw new Exception("Article not found"); // ZLE! Powinnismy zwrocic po prostu null
}
2. Nie należy również umieszczać w klauzuli Try\Catch zbyt dużej ilości kodu. Po pierwsze nie jest to optymalne rozwiązanie. Ważniejszym jednak powodem jest fakt, że wyrzucony i obsłużony wyjątek powinien dotyczyć prostych (pojedynczych) operacji. Zbyt ogólna informacja nie daje żadnych korzyści podczas debuggowania lub przeglądania wygenerowanych logów.
3. Rozważmy następujący fragment aplikacji:
try
{
// jakas logika
}
catch(Exception ex)
{
// obsluga wyjatku
}
Typ Exception jest najbardziej ogólnym wyjątkiem. Powinniśmy raczej łapać poszczególne typy wyjątków i uzależnić od nich obsługę np:
try
{
//
}
catch(ConnectionException ce)
{
// nie mozna nawiazac polaczenie, moze warto zaimplementowac mechanizm recovery?
}
catch(Exception e)
{
// Nie wiadomo do konca co sie stalo - wykonujemy odpowiedni wpis
// w pliku logow i np. zamykamy aplikacje
}
Drugi Catch lapie wszystko, pierwszy tylko szczególny typ wyjątku.
4. Łapanie wyjątku i ignorowanie go:
try
{
// logika
}
catch(Exception e}
{
// brak jakiegokolwiek kodu
}
Powyższy kod jest skrajnie zły – ignorując wyjątek pozwalamy aplikacji nadal funkcjonować i zapominamy po prostu, że to co chciał wykonać użytkownik tak naprawdę nie zostało wykonane. Skoro łapiemy wyjątek powinniśmy przynajmniej wykonać log.
6. Pomijanie stacktrace:
try
{
// logika
}
catch(Exception e)
{
// jakas logika, wykonanie logow itp.
throw e;// ZLE!, throw new Exception("msg") rowniez jest niepoprawne
}
Co się stanie z stacktrace? W momencie ponownego wyrzucenia, nowy zostanie wygenerowany z klauzuli catch. Stary zostanie po prostu usunięty i nie będziemy w stanie zobaczyć, w której metodzie tak naprawdę został wyrzucony wyjątek. Prawidłowe podejście, przechowujące prawidłowy stacktrace:
try
{
// logika
}
catch(Exception e)
{
// jakas logika, wykonanie logow itp.
throw;
}
Ewentualnie:
try
{
// logika
}
catch(Exception e)
{
// jakas logika, wykonanie logow itp.
throw new WrappedException("custom par 1",e);
}
W pewnych sytuacjach ukrywanie stacktrace ma sens – np. gdy nie chcemy dostarczyć hacker’owi jakiś informacji. Jednak myślę, że jest to rzadki przypadek dla większości z nas ( tak jak napisałem kiedyś, dostarczając bibliotekę DLL już dajemy potencjalnemu włamywaczowi sporo informacji).
To kilka pospolitych błędów. Jeśli macie jeszcze jakieś inne “kwiatki” to zachęcam do komentowania!