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
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
class List{
nullptr
next = 0;
const&
void pushBack(T value);
Używaj listy inicjalizacyjnej zamiast przypisywania w ciele konstruktora.
Lepiej mieć wszystko w nagłówku, niż sztucznie podzielić a potem zrobić
#include "list.cpp"
item *temp =first;
Raz masz spację po gwiazdce, raz nie masz. Tak samo ze znakiem =
.
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łofor (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ć.
if (first == 0)
if (!first)
szczególnie, że gdzie indziej masz
while (temp)
#ifndef
to po co #pragma once
.Dzięki za wnikliwą analizę.
Kod już poprawiłem, ale mam jeszcze dwa pytania:
twonek napisał(a):
- 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
?
- Zazwyczaj ludzie spodziewają się szybkiego
size()
, o czasie stałym. Zauważ, żestd::list
miało niezdefiniowaną złożoność metodysize()
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łofor (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.
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--;
}
Jak nie chcesz się bawić w new
i delete
, to zawsze jest unique_ptr
.