Code review: lista danych

Rozważmy następujący kod:

private IList<int> ParseString(string text)
{
  string[] numbersStr = text.Split(';');
  var numbers=new List<int>();

  foreach (string numberStr in numbersStr)
  {
      numbers.Add(Int32.Parse(numberStr));
  }
  return numbers;
}

Co jest największym problemem tego kodu? Wszystkie przykłady wymyślam na bieżąco dlatego oprócz głównego problemu, który chce zaprezentować w poście, istnieje kilka pobocznych. Na przykład:

1.  Średnik w funkcji Split nie powinien być zahardcodowany.

2. Zamiast Int32.Parse warto zastanowić się nad sprawdzeniem czy na pewno mamy do  czynienia z liczbą.

3. Prawdopodobnie użycie LINQ byłoby bardziej czytelne.

4. Warto zastanowić się czy nie lepiej zwracać IEnumerable zamiast IList.

Jak wspomniałem jednak, stworzyłem taki kod aby pokazać naprawdę duży problem, który jest często popełniany już w produkcyjnym kodzie.

Jak działa dynamiczna kolekcja danych List? Tak naprawdę jest to niestety zwykła tablica, która zmienia swój rozmiar jeśli to tylko potrzebne. Początkowo tablica ma pojemność 0. W momencie dodania pierwszego elementu pojemność ustawiania jest na 4. Po przekroczeniu 4 rozmiar jest podwajany do 8, potem do 16 itp. Realokacja tablicy jest bardzo wolnym procesem, ponieważ należy stworzyć nową tablicę a potem element po elemencie  skopiować zawartość. Jeśli znamy rozmiar docelowy tablicy warto przekazać pojemność w konstruktorze:

private static IList<int> ParseString(string text)
{
  string[] numbersStr = text.Split(new[]{';'},StringSplitOptions.RemoveEmptyEntries);
  var numbers=new List<int>(numbersStr.Length);

  foreach (string numberStr in numbersStr)
  {
      numbers.Add(Int32.Parse(numberStr));
  }
  return numbers;
}

W powyższy sposób uniknęliśmy ciągłej realokacji. Ponadto nie musimy alokować pamięci, której nie potrzebujemy – pamiętajmy, że automatycznie rozmiar jest podwajany.

Aby udowodnić to napiszmy jeszcze prosty programik pokazujący jak zmienia się pojemność listy:

 var numbers = new List<int>();

  Console.WriteLine(numbers.Capacity); // Pojemność: 0

  numbers.Add(1);

  Console.WriteLine(numbers.Capacity); // Pojemność: 4

  numbers.Add(1);
  numbers.Add(1);
  numbers.Add(1);
  numbers.Add(1);

  Console.WriteLine(numbers.Capacity); // Pojemność: 8

  numbers.Add(1);
  numbers.Add(1);
  numbers.Add(1);
  numbers.Add(1);

  Console.WriteLine(numbers.Capacity); // Pojemność: 16

Z jawną alokacją wygląda to następująco:

var numbers = new List<int>(16);

Console.WriteLine(numbers.Capacity); // Pojemność: 16

numbers.Add(1);

Console.WriteLine(numbers.Capacity); // Pojemność: 16

numbers.Add(1);
numbers.Add(1);
numbers.Add(1);
numbers.Add(1);

Console.WriteLine(numbers.Capacity); // Pojemność: 16

numbers.Add(1);
numbers.Add(1);
numbers.Add(1);
numbers.Add(1);

Console.WriteLine(numbers.Capacity); // Pojemność: 16

6 thoughts on “Code review: lista danych”

  1. A czy z listą nie jest tak, że jest to struktura dowiązaniowa i do elementów listy przypisywane są referencje do poprzedniego i następnego elementu? Tak przynajmniej było z listami czy wektorami w C++

  2. @kuba, to o czym mówisz to LinkedList zachowuje się tak jak napisałeś. Czasem rzeczywiście lepiej użyć jej zamist zwykłeś List. Znajduje się w tym samym namspece co zwykłe List czyli System.Collections.Generic.

  3. Dlaczego ta funkcja zwraca interfejs? Rozumiem że interfejsy dobrze jest stosować np. jako parametry funkcji, ale co można zyskać w tym przypadku?

  4. Hej

    Kilka uwag :-).

    “1. Średnik w funkcji Split nie powinien być zahardcodowany.”

    W kontekście twojego przykładu nie widzę też powodu by nie był, zmienna by tylko zaciemniała metodę, jeśli natomiast chciałbyś mieć możliwość zmiany separatora to lepiej zrobić dwie metody jedną przeciążoną gdzie można podać separator.

    “3. Prawdopodobnie użycie LINQ byłoby bardziej czytelne.”

    Zgadzam się natomiast wolniejsze, warto się zastanowić czy stać nas na dodatkowy narzut, lepiej już użyć czystej lambdy.

    Zamiast listy lepiej jest użyć tablicy, ponieważ nie alokuje ona tyle pamięci i dla prostych zadań jest zwyczajnie szybsza.

    Dodam jeszcze że dla dużych stringów, oraz relatywnie dużej ilości wywołań takiej metody podejście do zaprojektowania jej musi być *kompletnie* odmiennie niż napisałeś.

  5. @badamczewski:
    1.Każda liczba lub napis nie powinien być zahardcodowany – stała daje przede wszystkim opis i może być użyta w kilku miejscach. To dobry zwyczaj pisac kod samo-komentujacy sie…
    3. LINQ i wydajnosc… Wszystko zalezy od konkretnego przykladu ale zwykle nie ma to znaczenia…
    Jesli chodzi o tablice – tak w tym przykladzie lepsza byloby ale post jednak ma pokazac wady listy nieprawidlowo uzytej:)

    Dzieki za uwagi!

Leave a Reply

Your email address will not be published.