Code Review: przekazywanie parametrów

Dziś prosta zasada przekazywania parametrów ale jednak często łamana. Kod:

private static void Display(string[] strings)
{
   foreach (string text in strings)
   {
       Console.WriteLine(text);
   }
}

Powyższa metoda ma za zadanie wyświetlenie wyłącznie elementów. Parametr wejściowy (tablica string’ów) jest zbyt specyficzny i nie pozwala na przekazanie wszystkich zbiorów danych. Na przykład poniższy kod nie skompiluje się:

List<string> elements=new List<string>();
elements.Add("A");
elements.Add("B");

Display(elements);

Z tego względu zawsze należy używać bazowych interfejsów. W tym przypadku jest to IEnumerable:

private static void Display(IEnumerable<string>strings)
{
   foreach (string text in strings)
   {
       Console.WriteLine(text);
   }
}
private static void Main(string[] args)
{
   List<string> elements=new List<string>();
   elements.Add("A");
   elements.Add("B");

   Display(elements);
}

Ta sama zasada dotyczy się wszystkich typów klas – nie tylko kolekcji. Zamiast używać FileStream lepiej użyć bazowej klasy Stream, którą można użyć nie tylko dla zapisu do plików ale również do kompresji, transmisji danych przez sieć  itp.

// zła deklaracja
private static void CopyData(FileStream stream)
{        
}
// zła deklaracja
private static void CopyData(NetworkStream stream)
{
}
// poprawna, generyczna deklaracja
private static void CopyData(Stream stream)
{
}

IEnumerable nie zawsze jednak da się wykorzystać ponieważ np. dana funkcja musi dodać jakieś elementy do kolekcji. IEnumerable czy tablice są kolekcjami tylko do odczytu. BARDZO jednak częstym błędem jest pisanie kodu w następujący sposób:

private static void AddSomeItems(List<string> list)
{ 
}

A co jeśli użytkownik ma swoją własną kolekcję danych np. SortedList? Oczywiście nie będzie możliwe skorzystanie z naszej metody AddSomeItems. Z tego względu zawsze należy używać interfejsów w sygnaturach tzn.:

private static void AddSomeItems(IList<string>list)
{ 
}

W wielu przypadkach również typ zawracany lepiej deklarować jako interfejs:

private static IList<string> CreateList()
{      
}

W taki sposób łatwiej będzie w przyszłości zmienić wewnętrzną implementację CreateList bez łamania wszystkich zależnych bibliotek.

W niektórych jednak przypadkach lepiej zwracać typ specyficzny niż ogólny np.:

private static SqlCommand CreateSqlCommand()
{      
}
private static Command CreateSqlCommand()
{
}

Które rozwiązanie jest bardziej elastyczne? CreateSqlCommand tworzy specyficzne dla bazy Sql Server polecenie. Zwracanie ogólnego Command nie ma sensu.  Dostarczenie użytkownikowi SqlCommand daje mu dużo wyższą elastyczność – może dokonywać specyficznych dla SqlServer operacji. Implementacja wewnętrzna metody nigdy nie zmieni się do tego stopnia, że zwracany będzie inny typ.

14 thoughts on “Code Review: przekazywanie parametrów”

  1. Z jednym bym polemizował, mianowicie “private static void AddSomeItems(List list)”, uważam że Twoja uwaga jest zasadna dla metod nie prywatnych. Ogólnie wewnątrz klas lepiej się nie ograniczać i użyć List choćby po to aby móc wykonać list.AddRange(…).

  2. Zgadza sie. Dotyczy to glownie metod publicznych. Implementaca wewnetrzna (private) ma prawo zmienic sie.

  3. “W wielu przypadkach również typ zawracany lepiej deklarować jako interfejs” to nie jest prawdą, zasada brzmi by przyjmować typ jak najbardziej ogólny, a zwracać typ jak najbardziej szczegółowy.

  4. Ale ja przeczytałem i nadal podtrzymuję moje stanowisko by zawsze zwracać typ jak najbardziej szczegółowy. Jak zwrócimy “interfejs” i będziemy chcieli skorzystać jednak z pełnej
    funkcjonalności zwracanego obiektu to będziemy musieli robić rzutowanie, a na dodatek zgadnąć jakiego typu jest obiekt, a co gorsza typ obiektu może się zmieniać w kolejnych wersjach kodu! 😉

    “W taki sposób łatwiej będzie w przyszłości zmienić wewnętrzną implementację CreateList bez łamania wszystkich zależnych bibliotek.”
    Odnośnie tego, to zasada brzmi by korzystać z lekkich interfejsów, a nie ciężkich obiektów, więc to co odbieramy (jak najbardziej szczegółowe) pakujemy do minimalnego interfejsu który potrzebujemy.

  5. “Odnośnie tego, to zasada brzmi by korzystać z lekkich interfejsów, a nie ciężkich obiektów, więc to co odbieramy (jak najbardziej szczegółowe) pakujemy do minimalnego interfejsu który potrzebujemy”

    Czyli chcesz zwracac jednak interfejs?Interfejs to obiekt ogolny a nie specyficzny.

    Druga sprawa. Na dole postu napisalem, ze w zaleznosci od przypadku czasami lepiej zwracac specyficzny a czasami ogolny.

  6. Nie, chce zwracać obiekt specyficzny, a odbierać jako ogólny (minimalny który potrzebujemy w danym momencie). Wtedy znika problem z tymi zależnościami, bo zależności są minimalne, co implikuje, że zawsze należy zwracać jak najbardziej szczegółowy.

    interface IA { }
    interface IB { }
    class MyClass : IA, IB { }

    MyClass FactoryMethod()
    {
    return new MyClass();
    }

    IA a = FactoryMethod();
    IB b = FactoryMethod();
    MyClass c = FactoryMethod();

  7. Zgadza sie – nalezy odbiera jako ogolny.
    Niestety i tak w Twoim przypadku sygnatura sie zmieni a skoro uzywasz interfejsow to i tak bedzie musial castowac – przyklad IList,List i AddRange. Nie tworzymy w koncu dodaktowego interfejsu dla jednej metody, wykorzystywanej w jednej funkcji tylko – bedzie to bardzo nieczytelne.

    Jesli jest to prawdpodobne, ze uzytkownik bedize chcial uzywac specyficzny obiekt wtedy zwracamy specyficzny. Jednak w przypadku List bezpiecznie zwrocic IList niz List. Wszystko zalezy od przypadku, stwierdzenie ze zawsze nalezy zwracac obiekt specyficzny jest tak samo niepoprawne jak stwierdzenie, ze zawsze nalezy zwracac obiekt ogolny.

  8. Aj, nadal nie rozumiesz, my nie wiemy w jaki sposób użytkownik będzie chciał używać nasz obiekt, dlatego zwracamy najbardziej specyficzny.

    Użytkownik wie że będzie chciał użyć AddRange, to odbierze od nas obiekt specyficzny List.

    2 użytkownik będzie potrzebował tylko listę do odczytu, to odbierze jako IList.

    0 rzutowań! W obu przypadkach użytkownik wybiera minimalny zestaw który potrzebuje.

  9. @eN:
    Ale to nie zmienia faktu, ze kod bedzie zlamany po tym jak zmienia sie implementacja wewnetrzna metody. I to jest argument, dlaczego w niektorych przypadkach zwrocic IEnumerable a nie np. List. Moim zdaniem trzeba kierowac sie danym scenariuszem a nie “regulami”, ktore nie pokrywaja wszystkich przypadkow w realnej pracy z kodem.

  10. Nie wiem jak bardziej obrazowo to przedstawić, ta reguła pokrywa wszystkie przypadki, ale Ty tego jeszcze nie widzisz (:

    W metodzie zwracasz jak najbardziej specyficzny obiekt który jesteś w stanie w tej metodzie wytworzyć (powiedzmy 7 w kolejności dziedziczenia od klasy object), odbierasz go jako jak najbardziej ogólny, który zapewnia minimum które dany kod zapewnia(powiedzmy 2 w kolejności dziedziczenia, niżej zejść nie możemy). I mamy też kod który wymaga co najmniej 4 w kolejności dziedziczenia.

    Jeśli będziesz chciał teraz zmienić implementację z 7. to możesz zejść do maksymalnej wartości z minimalnych, czyli 4, i nie będziesz mógł zejść poniżej 4, bo tą 4 wymaga jakąś funkcjonalność której nie oferują klasy poniżej 4 w hierarchii dziedziczenia.

  11. Podsumowując, Twój obiekt musi spełniać maksimum z minimalnych potrzeb osób z niego korzystających, więc to oni ustalają minimum a nie Ty. Ty dajesz maksimum jakie możesz zapewnić, a czy możesz zejść później niżej, o tym nie Ty decydujesz, tylko kod któy korzysta z Twojego obiektu.

  12. Ok postaram sie o przyklad aby to rozjasnic.
    Powiedzmy ze mamy biblioteke A, ktora posiada nastepujaca metode:

    List GetUsers()
    {
    }
    Teraz uzytkownik biblioteki B moze cos takiego zrobic:

    var list=GetUsers(); // List.
    Nastepnie w bibliotece A zmieniamy sygnature na:
    SortedList GetUsers();

    SortedList implementuje IList ale nie dziediczy po List.
    Uzytkownik ma zlamany kod pomimo, ze mozna byloby tego uniknac
    Jesli chce uzywac AddRange to na wlasna odpowiedzialnosc. Zauwaz rownie ze czesto klasy nie dostarczaja dodatkowych metod a jedynie implementuja metody interfejsu. W takim przypadku zwracanie specyficznego typu nie ma zadnego sensu. IDENTYCZNY efekt mozna uzyskac zwracajac interfjes a mamy pewnosc, ze kod zawsze bedzie kompatybilny.

  13. Uważam, że Piotr ma rację, a nie nie eN. Typ najbardziej szczegółowy może sobie zwracać prywatna funkcja klasy, lub wewnętrzna w bibliotece. Jeśli tworzymy natomiast API dla klienta, to powinno się używać bardziej abstrakcyjnych typów (interfejsów), stosownie do potrzeb. Zwracanie typów szczegółowych to poderżnięcie gałęzi na której się siedzi. Wcześniej czy później nadejdzie czas zmian, a my trzymamy rękę ciągle w tym samym nocniku. I nic nie możemy zrobić, bo kontrakt API na to nie pozwala.

Leave a Reply

Your email address will not be published.