Vracet List je špatné

08.51 - 1. srpna 2009 | ASP.NET 2.0

Přinejmenším na veřejnosti. Mnohdy se v různých APIs můžete setkat s tím, že různé objekty veřejně vystavují metody/vlasnosti, které mají návratový typ List<T>, v lepším případě IList<T>. Krásná ukázka nerozvážného návrhu.

Proč je špatné vracet `List<T>`

Když vracíte List<T>, znamená to, že říkáte všechno o vnitřní implementaci a zavíráte si vrátka pro její možnou změnu. Generický List sice neni nerozšiřitelný (sealed), ale ani nebyl při návrhu moc k rozšiřování zamýšlen. Navíc tim porušujute jeden ze základních principů OOP – a to zapouzdření. Dále tim umožňujete konzumentovi vaši kolekci přímo měnit.

Ukázka naivního/lenošného přístupu

Pro ukázku mějme třídu reprezentující uživatele User, kterému můžeme přiřadit uživatelské role Role. Naivní přístup:

public class User {
  public List<Role> Roles { get; private set; }
}

public class UserConsumerCode {
  public void SomeMethod {
    var user = new User();
    user.Roles.Add(Role.Admin);

    // ...

    user.Roles.Remove(Role.Reviewer);

    // ...

    var isAdmin = user.Roles.Contains(Role.Admin);
  }
}

Jak vidíte, klient má nad kolekcí rolí uživatele plnou moc. Může si přidávat, odebírat, přetřizovat jak se mu zlíbí. A my nemáme nejmenší šanci tyto celkem důležité operace jakkoli ovlivnit, nebo u nich vědět. Kdybychom změnili typ kolekce Roles na IList<Role>, tak sice můžeme změnit vnitřní implementaci a místo List<T> použít třeba LinkedList<T>, ale stále zveřejňujeme příliš mnoho. Jedinou výhodou tohoto řešení je, že jsme nemuseli napsat moc kódu a máme hodně funkcionality.

Jak zlepšit náš kód?

Rovnou přeskočím použití ICollection<T>, a přejdeme k tomu, jak by to mělo vypadat. Zachováme si zapouzdření, uzavřeme se vůči nechtěné manipulaci z vnějšku a otevřeme se snadným vnitřním změnám a rozšiřitelnosti (Open/closed principle).

public class User {
  private readonly ICollection<Role> _roles;

  public User() {
    _roles = new List<Role>();
  }

  public IEnumerable<Role> Roles {
    get { return _roles.ToArray(); }
  }

  public void AddRole(Role role) {
    _roles.Add(role);
  }

  public void RemoveRole(Role role) {
    _role.Remove(role);
  }

  public bool IsInRole(Role role) {
    return _roles.Contains(role);
  }
}

Pěkně jsme obohatili a zpřehlednili naše API (zápis user.IsInRole(Role.Admin) říká mnohem více, než user.Roles.Contains(Role.Admin), navíc neporušuje Demeterův zákon), skryli jsme implementační detaily a zároveň nechali otevřená vrátka pro snadnou výměnu vnitřní implementace… Dalším krokem by měly být kontrakty manipulačních metod, aby byly bezpečné. Dále pak můžeme přidat vyvolávání událostí, abychom o manipulaci věděli. Míst pro rozšíření je tu spousta.

Hloupá otázka na závěr

„Ale, jak teď zjistim, kolik rolí má uživatel přiřazených, když IEnumerable nemá vlastnost Count ani Length?“ using System.Linq; ;)

Komentáře

Michal

09.10 - 1. srpna 2009 | #

Tato strategie je vhodna tehdy, pokud potrebujes do budoucna vnitrni implementaci skryt nebo se ocekavaji vetsi zmeny ve vnitrni implementaci (anebo je nezadouci, aby mohl konzument s datama snadno manipulovat).

Pokud se pracuje s treba vysledkem dotazu, seznamem uzivatelu podle pevnych kriterii (a zmeny v kolekci nemaji dopad kamkoliv jinam), pak je IList/List/kolecke apod. prijemne zjednoduseni zivota na obou stranach.

Ale dobry clanek, spouste lidi tebou prezentovana situace nedochazi a to je spatne. A obecne by API melo spise skryvat, nez odkryvat ;-)
Aleš Roubíček

09.32 - 1. srpna 2009 | #

[1] Michal: Ani u výsledků dotazů není důvod List/IList zveřejňovat. Dnes už neni situace, kde by nestačilo IEnumerable a LINQ. :) A pokud jako konzument ten List opravdu nutně potřebuješ, pak má pěkný konstruktor, který ho z IEnumerable vytvoří.

Ad API: To neni o skrývání, ale o usměrňování.
Tomáš Jecha

17.29 - 1. srpna 2009 | #

Nebylo by lepší podědit od ReadOnlyCollection a implementovat mu metody na přidání a odebrání rolí? Třída User jej pak bude nabízet vlastností Roles. Proč přetahovat logiku kolekce přímo do objektu uživatele?
Aleš Roubíček

18.04 - 1. srpna 2009 | #

[3] Tomáš Jecha: Ano, nebylo. User totiž nemá logiku kolekce. To si myslíš jen díky tomu, že znáš implementační detaily. Ty se ovšem mohou v průběhu času měnit (a v praxi se mění).

V prvním případě, s takovou změnou budeš mít hodně práce. Všude, kde pracuješ s listem rolí user.Roles, budeš muset udělat změnu. Pokud tvůj kód konzumuje i někdo jiný, máš zaděláno na BC a ty většinou nikdo rád nemá. V druhém případě takovým nepříjemnostem předcházíš.

Navíc je snad pochopitelné, že když chci přidat uživateli roli user.AddRole, tak to neznamená, že chci přidal doli do kolekce, která patří uživateli user.Roles.Add. BTW přečti si v článku odkazovaný Demeterův zákon, tam je přesně popsáno, proč má odpověď na tvou první otázku je kladná.
Tomáš Jecha

18.37 - 1. srpna 2009 | #

[4] Aleš Roubíček: O to jde, mě se právě onen „Demeterův zákon“ moc nelíbí. Seznam rolí je jednou seznam a schovávat ho za metody do rodičovského objektu mi přijde jako krok zjednodušení objektového návrhu za cenu ztráty obecného přístupu ke kolekci. Podle mě se vhodnost použití obou přístupů značně liší případ od případu.
Aleš Roubíček

20.49 - 1. srpna 2009 | #
[5] Tomáš Jecha: Pozor na to, seznam rolí, rozhodně nemusí být kolekce rolí!
```
public class User {
  private string _roles;

  public User() {
    _roles = String.Empty;
  }

  public void AddRole(Role role) {
    if (String.IsNullOrEmpty(_roles) == false) {
      _roles += " ";
    }
    _roles += role.Name;
  }

  public void RemoveRole(Role role) {
    _roles.
      Replace(role.Name, String.Empty).
      Replace("  ", " ");
  }

  public bool IsInRole(Role role) {
    return _role.Contains(role.Name);
  }
}
```
Tato implementace nepoužívá žádnou kolekci a je stejně validní jako implememntace s listem rolí. Je to jen implementační detail. Objektový návrh by měl být co nejjednoduší. O to přece jde, dělat věci jednoduše, intuitivně, ne něco lámat přes koleno. Jak už jsem psal v minulé reakci, jde o to přiřadit roli uživateli, ne o to přidat záznam do nějaké kolekce.

Můj příklad byl možná špatný v tom, že jsem publikoval vlastnost IEnumerable<Role> Roles. V podstatě by měly bohatě statčit ony tři metody AddRole, RemoveRole a IsInRole. Nic víc…
Augi

21.19 - 1. srpna 2009 | #

Pěkný. Vracet IEnumerable<T> jsem se naučil už dávno. Předtim jsem měl ještě nějaký techtle-mechtle s ReadOnlyCollection, ale ty jaksi nedopadly. Vracet IEnumerable<T> má taky tu výhodu, že v metodě můžeme vesele yieldovat ;-) A ještě bych vypíchl nedoceněné použití ToArray() – dokáže to vyřešit pár základních problémů – multi-threading a úpravu kolekce během enumerace.
Aleš Roubíček

22.01 - 1. srpna 2009 | #

[7] Augi: Přesně o tom bude zítřejší spot, pokud nebude velká kocovina :)
Rene Stein

22.20 - 1. srpna 2009 | #
Jen rychla poznamka z MDA. Tento postup je hezky, ale v prazi rychle narazis na limity. Podle me jde v clanku o „skolni“ priklad – na prvni pohled pekne reseni ukazujici vyhody, ale trochu nucene a nepresvedcive.
1. Demeteruv zakon – slovo zakon je nestastne, jedna se spis o uzitecne doporuceni, jak kontrolovat, ze implementace metod nevede ke vzrustajicimu zatizeni tridy dalsimi nesouvisejicimi tridami, a tim i k likvidaci jednoho z hlavnich zaklinadel OOP, tedy znovupouzitelnosti tridy, „cerne skrinky“, ktera nema prilis predpokladu o ostatnch tridach v okoli.
2. Co kdyz trida uzivatel bude obsahovat i kolekci vlastnich prav. Bude vystavena kolekce? Objekt, na ktery je delegova sprava prav? Nebo ty vystavis dalsi (treba jen delegujic na spravce pravi) metody AddRight, RemoveRight, HasRight? Rozhrani sice bude pekne, ale po case se pri tomto postupu zacnes potykat se zasadou"rozhrani tridy by melo byt jednoduche a intuitivni, nemely byste mit ve tride prilis mnoho metod". Neni tedy potreba jeste odkazat na princip separatnich rozhrani tridy (ISG)? A jsme schopni „rozseka“ rozhrani tridy User tak, aby jednotliva specializovana rozhrani slouzila ruznym klientum?
Tim chci jen rici, ze navrh je (neprekvapive) kompromisem mezi ruznymi zakony a principy a ze napsat „kontroverzni“ i clanek, jak jsi zminoval na Twitteru, se da vzdy nejlepe pojetim nejakeho principu jakozto axiomu vsech axiomu a kladiva s totalnim narokem na ubiti vsech problemu. ;)
1. List(T) pro typove kolekce nepouzivam. Mam vlastni typove kolekce zalozene na Collection(T).
Omlouvam se za preklepy, na MDA se ale komentar do tohoto boxu pise spatne. ;)
stej

10.05 - 3. srpna 2009 | #

„Ale, jak teď zjistim, kolik rolí má uživatel přiřazených, když IEnumerable nemá vlastnost Count ani Length?“ using System.Linq;

Osobně se ztotožňuju s IEnumerable.Count() is a Code Smell
Aleš Roubíček

10.27 - 3. srpna 2009 | #

[10] stej: Samozřejmě, v tamtom případě, ano. Jenže to, co popisuju já, negeneruje dotaz do databáze. :) Pokud programátor, netuší, co jeho kód dělá, měl by přemýšlet o jiném zaměstnání.
Steve

20.33 - 29. srpna 2009 | #

[11] Aleš Roubíček: Kdychom to měli vzít důsledně, tak na msdn se o IEnumerable píše „Exposes the enumerator, which supports a simple iteration over a (non-)generic collection.“ => IEnumerable (schválně) „neexposuje“ nic o počtu prvků.

Dogmatické řešení dovedené do krajností by podle mě mělo vypadat tak, že třída User bude mít ještě vlastnost RolesCount.

Btw. a co třeba RolesCollection implementující IList<> (ICollection<>) a vnitřně využivající List<> pomocí skládání? :-P

Jinak taky vždy vracím IEnumerable<>, ale teprve díky tomuhle článku a komentářům pod ním, jsem o tom začal hlouběji přemýšlet.
Aleš Roubíček

08.31 - 30. srpna 2009 | #

[12] Steve: Díky za reakci. :)

Já jsem s odstupem času došel k tomu, že příklad s uživatelskými rolemi, nebyl zrovn nejšťastnější. :) Lepší by byla ukázka ve stylu person.Wallet.Remove(), kde Wallet je IList of Money vs. person.PayBill(). :) Myslím, že šahat na peněženku si jen tak někdo nenechá. A pak je Demeterův zákon stravitelnější.

Ano RolesCount by šlo implementovat, ale opravdu potřebuje uživatel znát, kolik rolí má?
Steve

23.16 - 30. srpna 2009 | #

[13] Aleš Roubíček: Souhlasím: peněženka dává větší smysl, tam je to pěkně vidět.

Ad potřebnost RolesCount: záleží na tom. Když bych např. chtěl v UI administrace umožnit editaci seznamu rolí přiřazených danému uživateli, tak by IEnumerable<Role> i RolesCount (resp. Linq) přišli vhod pro jejich výpis, stránkování atd.

Taky díky za reakci :)