Lista jednokierunkowa

0

Chciałbym poddać krytyce moją listę jednokierunkową.
https://github.com/lukaszres/List

Na razie można na niej wykonać operacje:
size()
pushBack()
get()
set()
Za wszelkie uwagi będę wdzięczny

1
  1. Pierdoła, kwestia gustu itd. ale spodziewałbym się spacji przed klamrą
class List{
  1. Używaj nullptr
next = 0;
  1. Przekaż parametry jako const&
void pushBack(T value);
  1. Używaj listy inicjalizacyjnej zamiast przypisywania w ciele konstruktora.

  2. Lepiej mieć wszystko w nagłówku, niż sztucznie podzielić a potem zrobić

#include "list.cpp"
  1. Trzymaj się konsekwentnie swojego stylu formatowania
item *temp =first;

Raz masz spację po gwiazdce, raz nie masz. Tak samo ze znakiem =.

  1. Zazwyczaj ludzie spodziewają się szybkiego size(), o czasie stałym. Zauważ, że std::list miało niezdefiniowaną złożoność metody size() przed C++11, więc mogła być liniowa, ale od C++11 musi być stała. Pomyśl jak długo by to trwało
for (unsigned int i = 0; i< myList.size(); i++)

jeśli kompilator nie zoptymalizuje wywołania myList.size() tylko w każdym obiegu pętli będzie to wołać.

  1. Konsekwencja
if (first == 0)
if (!first)

szczególnie, że gdzie indziej masz

while (temp)
  1. Jeśli masz #ifndef to po co #pragma once.
0

Dzięki za wnikliwą analizę.
Kod już poprawiłem, ale mam jeszcze dwa pytania:

twonek napisał(a):
  1. Lepiej mieć wszystko w nagłówku, niż sztucznie podzielić a potem zrobić
#include "list.cpp"

Tzn. masz na myśli, żeby wszystkie ciała metod przełożyć do pliku list.h i całkiem zlikwidować list.cpp?

  1. Zazwyczaj ludzie spodziewają się szybkiego size(), o czasie stałym. Zauważ, że std::list miało niezdefiniowaną złożoność metody size() przed C++11, więc mogła być liniowa, ale od C++11 musi być stała. Pomyśl jak długo by to trwało
for (unsigned int i = 0; i< myList.size(); i++)

jeśli kompilator nie zoptymalizuje wywołania myList.size() tylko w każdym obiegu pętli będzie to wołać.

A może utworzyć w klasie nowe pole size i przy wywołaniu konstruktora struct item zwiększać je?
Wtedy metoda size() będzie tylko zwracać wartość pola size i złożoność będzie stała.

0

Dodałem funkcję erase(), tylko delete item jakoś mało eleganckie mi się wydaje. Jakieś sugestie?

template <class T>
void List<T>::erase(unsigned int where)
{
    if (where >= this->size())
        throw std::string("err:: erase() position is out of range");
    item *temp = first;
    for (unsigned int i = 0; i<where-1; i++)
        temp = temp->next;
    item *toDelete =  temp->next;
    temp->next = temp->next->next;
    delete toDelete;

    this->counter--;
}
0

Jak nie chcesz się bawić w new i delete, to zawsze jest unique_ptr.

1 użytkowników online, w tym zalogowanych: 0, gości: 1