• Artykuł

     

Dobre i złe praktyki w C# - część I  

Udostępnij na: Facebook

Autor: Piotr Zieliński

Opublikowano: 2012-07-16

Wprowadzenie

Podstawy języka C# są stosunkowo proste i zdecydowanie intuicyjne. Znajomość samej składni języka to jedynie podstawa, która nie czyni z programisty osoby zaawansowanej w C#.  Warto zwrócić uwagę, że składnia tego języka podobna jest do składni innych języków, takich jak Java czy C++. Głównie dzięki dobrym praktykom oraz sposobom optymalizacji można odróżnić języki od siebie. Każdy język ma pewne wzorce, specyficzne dla niego samego, z którymi należy się zapoznać. Programiści Java napiszą kod inaczej niż programiści C#. Część osób mówi, że język to tylko narzędzie i nie ma znaczenia, jaki się wybierze, ponieważ składnia jest i tak podobna. Na poziomie podstawowym jest to prawda. Jeśli jednak kod ma być wydajny i czytelny, wówczas przyswojenie nowego języka jest dużym wyzwaniem, wymagającym mnóstwo czasu oraz doświadczenia. Artykuł stanowi luźny zbiór złych oraz dobrych praktyk, obejmujących rozmaite aspekty języka. Oprócz dobrych praktyk, specyficznych dla C#, zostaną zaprezentowane najbardziej powszechne wzorce oraz antywzorce, niezależne od konkretnego języka.

Nazewnictwo, notacja

Każdy zespół może ustalić własną notację. Istnieją jednak pewne zwyczaje, niezmienne w świecie .NET. Programiści przyzwyczajeni są do notacji znanej z .NET Framework oraz innych znanych frameworków. Z tego względu, nie ma sensu używać kompletnie odrębnej notacji, ponieważ dla innych osób będzie się to wydawało dziwne i mało czytelne. Przede wszystkim, klasy powinny zaczynać się z wielkiej litery i przestrzegać notacji PascalCase.

Poprawne przykłady:

class  FileManager{}
class Manager{}
class XmlFileParser{}

Błędne przykłady:

class fileManager{}
class manager{}
class FILE_MANAGER{}

PascalCase, czyli zaczynanie każdego wyrazu z dużej litery, jest notacją występującą w wielu miejscach w .NET. Metody (w przeciwieństwie do Javy) powinny zostać zapisanie również zgodnie z PascalCase.

Poprawne przykłady:

private Map GetMapFile()
private void SetData()

Błędne przykłady:

private void SETFILE() // Tylko pierwsza litera musi być duża.
private void setFile() // W Java byłoby to poprawne.

Ponadto, metoda musi być w formie czasownika w trybie rozkazującym.

Poprawne przykłady:

int ComputeDelta()
void FetchData(…)

Błędne przykłady:

int Delta()
int Data();
void Time()  // Powinno być GetTime. Time to poprawna nazwa, ale dla właściwości.

Parametry metod powinny rozpoczynać się z małej litery.

Poprawny przykład:

void GetFile(string filePath)

Błędne przykłady

void GetFile(string _filePath)
void GetFile(string FilePath)
void GetFile(string strFilePath) // Str jest zbędne.

Podobnie wygląda kwestia lokalnych zmiennych.

Interfejsy powinny zaczynać się od litery “I” a kończyć mogą się “able”, np.: IEnumerable czy IWritable.

Artykuł ma na celu pokazanie dobrych praktyk, a nie notacji w .NET. Zatem, w celu dalszej analizy, warto zajrzeć do opracowanych już artykułów, poświęconych wyłącznie notacji w .NET. Wnioskiem tego podpunktu jest fakt, że należy dostosować się do ogólnie znanej notacji, występującej w .NET Framework i w świecie .NET. Wprowadzenie nowej notacji, może spowodować zamieszanie dla nowych programistów, którzy przyzwyczajeni są już do .NET Framework. Każdy programista musi znać .NET Framework, więc składnia, znana z tego frameworka, jest dla niego po prostu bardziej zrozumiała. Na zakończenie, warto jednak dodać kilka ogólnych wskazówek, zalecanych przez Microsoft (źródło MSDN):

  • nie bój się długich nazw zmiennych! Dla czytelnika lepsza jest długa, choć opisowa nazwa niż krótka, wykorzystująca nieznane skróty. Preferuj czytelność, a nie wyłącznie zwięzłość,
  • nie używaj podkreślników w nazwach,
  • nie używaj notacji węgierskiej. W dzisiejszych środowiskach programistycznych tak dokładne opisywanie zmiennych nie ma sensu. Z punktu widzenia czytelnika, ważniejsza jest informacja o przeznaczeniu lub logice biznesowej, a nie czy zmienna jest typu float, czy int. Takie informacje z łatwością dostarczane są przez IDE.
  • nazwy powinny przede wszystkim opisywać rozwiązywany problem, a nie detale techniczne.

Gettery oraz Settery

Programiści przyzwyczajeni do Java lub CPP używają często klasycznych metod do enkapsulacji pól, na przykład:

class Person
{
    private string _firstName;
    public string GetFirstName()
    {
        return _firstName;
    }
    public void SetFirstName(string firstName)
    {
        _firstName = firstName;
    }
}

W C# służą do tego właściwości:

class Person
{
    public string FirstName { get; set; }
}

Nierzadko popełnianym błędem jest wstawianie zawansowanej logiki do właściwości. Właściwość powinna zwracać jedynie pole lub wykonywać jakieś proste obliczenia. Programista, widząc właściwość, może umieścić ją w pętli, co w przypadku skomplikowanej logiki znacząco spowolni działanie aplikacji. Przykład niebezpiecznej konstrukcji:

class Database
{
        public Employes[] Employees
        {
            get
            {
                WebService service = new WebService();                
                return service.GetAllEmployees();
            }
        }
}

Użytkownik może wywoływać Employess wielokrotnie, co oczywiście jest niewydajne. Czasochłonne operacje należy umieszczać w metodach o nazwach mówiących, że wynik będzie musiał zostać pobrany i nie jest dostępny natychmiast (np. FetchAllEmployees). Właściwości powinny zachowywać się jak zwykłe zmienne, do których dostęp jest natychmiastowy. Jednak, dopuszczalne jest wykonanie prostych obliczeń np.:

class Rectangle
{
    public int Area
    {
         get { return Width*Height; }
    }
}

Język jako opis procesów biznesowych

Nie powinno się umieszczać w kodzie liczb, które same nie mówią czytelnikowi za wiele, np.:

int _pageMode;
if (_pageMode == 2) 
{
    Response.Write("You are in edit mode");
} 
else if (_pageMode == 3) 
{
    Response.Write("You are in read only mode");
}

Wszystkie liczby należy opisać za pomocą stałych. Dla powyższego przykładu najlepszym rozwiązaniem jest użycie enum:

enum Mode {
Default, Edit, ReadOnly
}
Mode _pageMode;
if (_pageMode == Mode.Default)
{
    Response.Write("Default mode");
} 
else if (_pageMode == Mode.Edit)
{
    Response.Write("Edit mode");
}

Stosowanie magicznych liczb znacząco utrudnia późniejsze zrozumienie kodu:

int currentAttempt = 0;
while (currentAttempt < 5)
{
currentAttempt++;
}

Czytelnik nie ma pojęcia, co oznacza liczba „5”. Dodanie stałej ułatwia zrozumienie kodu:

int currentAttempt = 0;
const int maxAttemptsNumber = 5;
while (currentAttempt < maxAttemptsNumber )
{
currentAttempt++;
}

Należy zdać sobie sprawę, że nadawanie opisowych nazw jest dużo lepsze niż umieszczanie komentarzy. Kod powinien zostać tak prosto napisany, aby komentarze były po prostu zbędne.

Powyżej przedstawiono enum’y oraz const. Istnieje jeszcze modyfikator readonly. Warto zastanowić się nad różnicami między const a readonly.

Const:

  • wartość stałej podstawiana jest w czasie kompilacji. Zatem w kodzie binarnym nie ma stałej, wstawiana jest już konkretna liczba. Z tego względu, stałe są wydajne, bo nie alokują dodatkowej pamięci. Niestety, nie można używać typów, których nie da się stworzyć na etapie kompilacji - klasy, struktury itp.,
  • ze względu na powyższy punkt, nie można dodawać modyfikatora static,
  • Const można zainicjalizować wyłącznie na poziomie deklaracji,
  • stałe można wykorzystywać w klauzulach case (switch).

Readonly:

  • wartość podstawiana jest już w trakcie działania aplikacji. Readonly to normalne pole, które alokuje pamięć,
  • można go używać w połączeniu z modyfikatorem static,
  • może być zainicjalizowane jak stała na poziomie deklaracji oraz dodatkowo w konstruktorze.

Metody są również znakomitym sposobem na tworzenie opisowego kodu. Na przykład, zamiast umieszczać skomplikowane warunki, bezpośrednio w instrukcji if, lepiej przenieść kod do metody.

Niepoprawny kod:

if(position.X>0&&position.X<Width&&position.Y>0&&position.Y<Height)
{
}

Poprawny kod:

if(IsInsideControl(position))
{
}

Dobry kod nie potrzebuje komentarzy, a jego cechą charakterystyczną jest możliwość czytania go od ogółu do szczegółu. Jeśli programista musi zrozumieć detale, aby poznać ogólnie workflow klasy, wtedy jest to wyraźny sygnał, że kod musi zostać zrefaktoryzowany.

Obsługa błędów

Obsługa błędów może wydawać się prosta, np.:

try
{
_deviceConnection.Connect();
}
catch(Exception e)
{   
}
finally
{
}

Konstrukcja try-catch-finally powinna być wszystkim znana. Niestety, programiści często popełniają poważne błędy:

  1. Klasa Exception zawiera informacje o błędzie oraz tzw. StackTrace, przechowującym nazwy metod, które zostały wywołane. Pozwala to na identyfikacje scenariusza powodującego błąd. Dzięki StackTrace wiadomo, gdzie wyrzucony został wyjątek oraz jakie metody zostały kolejno wywołane. Utrata tych informacji znacząco utrudnia późniejszą analizę błędu. Poniższy kod jest dość częstym błędem:
try
{
 // logika
}
catch(Exception e)
{
// jakaś logika
throw e; // ŹLE!
}

W powyższym scenariuszu wyłapywane są wszystkie wyjątki. Następnie, wykonywana jest jakaś logika (np. wykonanie logów), a na końcu ponownie zostaje wyrzucony wyjątek, aby go później wyłapać w wyższej warstwie. Ponowne wyrzucanie wyjątku jest popularnym scenariuszem, jeśli obsługa niepowodzenia nie może odbyć się w całości w tej samej warstwie. Na przykład, wyjątek w warstwie dostępu do danych musi zostać wyłapany, aby móc zamknąć połączenie, ale musi zostać ponowie wyrzucony, żeby na końcu możliwa byłaby centralna rejestracja logów. Niestety, powyższa konstrukcja (throw e) jest niepoprawna, ponieważ gubi informacje o stacktrace. W celu wyjaśnienia, co dokładnie się dzieje, warto uruchomić następujący przykład:

internal class Program
{
        private static void Main(string[] args)
        {
            try
            {
                DoSomething();
            }
            catch (Exception e)
            {                
                Console.WriteLine(e.StackTrace);
            }            
        }
        private static void DoSomething()
        {
            try
            {
                FailSomething();
            }
            catch (Exception e)
            {
                throw e;
            }
        }
        private static void FailSomething()
        {
            throw new Exception();
        }
}

Kod wydrukuje na ekranie następujący StackTrace:

at ConsoleApplication4.Program.DoSomething() in C:\projects\ ConsoleApplication4\Program.cs:line 35
   at ConsoleApplication4.Program.Main(String[] args) in C:\projects\ ConsoleApplication4\ConsoleApplication4\Program.cs:line 19

Pytanie brzmi: gdzie podziała się informacja o FailSomething? Z StackTrace można wywnioskować jedynie, że w metodzie Main została wywołana DoSomething, która z kolei wyrzuciła wyjątek. Jak widać, nie pokrywa się to z rzeczywistością. Ponowne wyrzucanie wyjątku powinno wyglądać następująco:

private static void DoSomething()
{
        try
        {
            FailSomething();
        }
        catch (Exception e)
        {
            throw; // wcześniej było “throw e”;  
        }
}

Po tej drobnej zmianie StackTrace jest już pełny i poprawny:

at ConsoleApplication4.Program.FailSomething() in C:\projectsConsoleApplication4\ConsoleApplication4\Program.cs:line 39
   at ConsoleApplication4.Program.DoSomething() in C:\projects\ ConsoleApplication4\Program.cs:line 33
   at ConsoleApplication4.Program.Main(String[] args) in C:\projects\ ConsoleApplication4\Program.cs:line 18

Z powyższych eksperymentów wynika fakt, że StackTrace uzupełniany jest w momencie wyrzucania wyjątku za pomocą słowa kluczowego throw. Idąc tym tropem, można umieścić oryginalny wyjątek w innym wyjątku i wtedy StackTrace nie zostanie nadpisany:

internal class Program
  {
        private static void Main(string[] args)
        {
            try
            {
                DoSomething();
            }
            catch (Exception e)
            {
                Console.WriteLine(e.InnerException.StackTrace);
            }
        }
        private static void DoSomething()
        {
            try
            {
                FailSomething();
            }
            catch (Exception e)
            {
                throw new Exception("", e);
            }

        }
        private static void FailSomething()
        {
            throw new Exception();
        }
   }

InnerException zawiera macierzysty wyjątek z poprawnym StackTrace. Oba rozwiązania są poprawne – wszystko zależy od konkretnego przypadku użycia.

  1. Oczywiste jest, że łapanie wyjątku musi mieć jakiś cel. Mimo to, można czasami zauważyć następującą, niebezpieczną konstrukcję:
try
{
 // logika
}
catch(Exception e}
{
}

Pusty catch to po prostu ukrywanie błędów i udawanie, że wszystko jest w porządku. Innymi słowy, aplikacja nie zasygnalizuje błędu, mimo że jakaś operacja w tle nie została prawidłowo wykonana. Zachowanie bardzo niebezpieczne i trudne w analizie. Typowy przykład antywzorca. Jedynym prawidłowym scenariuszem jest sytuacja, w której nie możemy modyfikować zależnej biblioteki a nie dostarcza ona metod typu Int32.TryParse, które sprawdzą czy operację można wykonać.

  1. Dobrym zwyczajem jest łapanie konkretnych wyjątków, a nie ogólnego Exception. Nieelegancki kod:
try
{
    // jakaś logika
}
catch(Exception ex)
{
   // obsługa wyjątku
}
    Rozwiązaniem o większych możliwościach jest:
try
{
 //
}
catch(ConnectionException ce)
{
 // próba ponownego połączenia z usługą
}
catch(Exception e)
{
// Nie wiadomo do końca, co się stało - wykonujemy odpowiedni wpis
// w pliku logów i np. zamykamy aplikację.
}

Własne klasy wyjątków pozwalają na przekazywanie dodatkowych informacji. ConnectionException może dostarczyć informacje np. o usłudze, do której próbowano się połączyć lub o dokładniej przyczynie niepowodzenia. Łapanie ogólnego wyjątku nie daje tej elastyczności i ogranicza się zwykle do zapisania ogólnego log’u. Oczywiście, nie zawsze jest sens tworzenia własnych wyjątków, ale należy się nad tym zastanowić podczas projektowania.

  1. Przykład:
try
{
    connection.Connect();
    string serial = connection.GetSerialNumber();
    var points = connection.GetAllPoints();
    connection.SetAlarms();
}
catch(Exception e)
{
}

Powyższa próbka kodu prezentuje nadmierne nagromadzenie logiki w pojedynczym bloku try-catch. Należy pamiętać, że try-catch powoduje utratę wydajności, dlatego należy ograniczać ją do pojedynczych operacji. Dużo poważniejszym problemem jest jednak analiza wyrzucanego wyjątku i odpowiednia reakcja. Mając zbyt dużo logiki, nie wiadomo, na czym miałaby polegać obsługa błędu.

  1. Nadużywanie wyjątków. Wyjątek powinien być stosowany w naprawdę „wyjątkowych” sytuacjach. Jeśli metoda przeszukująca bazę danych nie znajdzie danego rekordu, nie powinno ono zakończyć się wyjątkiem. Niepoprawny przykład:
Article FindArticle(string text)
{
    Article article = ...
    if(article == null)
        throw new Exception("Article not found"); 
}

Zwykle, lepszym rozwiązaniem jest zwrócenie wartości NULL, o ile dopuszczalne jest nie znalezienie danego wiersza w bazie danych. Kolejny znany przykład to parsowanie tekstu:

int number;
try
{
    number = int.Parse(text);
}
catch
{
    number = -1;
}

Wystąpienie niepoprawnego tekstu nie jest czymś wyjątkowym. Szybszym i bardziej przejrzystym rozwiązaniem jest użycie metody TryParse:

int number;
if(int.TryParse(text, out number) == false)
    number = -1;

Zakończenie

Artykuł stanowi dopiero wprowadzenie do dobrych i złych praktyk w świecie C#. Kolejne części będą dotyczyć już bardziej specyficznych problemów, jak np. programowanie współbieżne. Uważałem jednak, że warto zacząć od podstaw, z których część ma zastosowanie również w innych językach. Przedstawione przykłady, prezentujące złe praktyki, widziałem niejednokrotnie, przeglądając kody open-source’owe. Należy pamiętać, że dobry kod jest łatwy w utrzymaniu, a inwestycja włożona w jego ulepszanie na pewno się w przyszłości zwróci. Pisanie złego kodu to po prostu przeznaczanie mniej czasu na projektowanie, a więcej na testowanie i naprawianie bugów. Co gorsze, po jakimś czasie staje się to bardzo kosztowe.

 

     

Piotr Zieliński

Absolwent informatyki o specjalizacji inżynieria oprogramowania Uniwersytetu Zielonogórskiego. Posiada szereg certyfikatów z technologii Microsoft (MCP, MCTS, MCPD). W 2011 roku wyróżniony nagrodą MVP w kategorii Visual C#. Aktualnie pracuje w General Electric pisząc oprogramowanie wykorzystywane w monitorowaniu transformatorów . Platformę .NET zna od wersji 1.1 – wcześniej wykorzystywał głównie MFC oraz C++ Builder. Interesuje się wieloma technologiami m.in. ASP.NET MVC, WPF, PRISM, WCF, WCF Data Services, WWF, Azure, Silverlight, WCF RIA Services, XNA, Entity Framework, nHibernate. Oprócz czystych technologii zajmuje się również wzorcami projektowymi, bezpieczeństwem aplikacji webowych i testowaniem oprogramowania od strony programisty. W wolnych chwilach prowadzi blog o .NET i tzw. patterns & practices.