Przy każdej uwadze dodam jak poważny jest to wg mnie błąd w skali od 1 do 6:
1. 2/6 poprawne nazewnictwo zwieksza czytelnosc kodu
template <typename data>
imo nazwa data troche myląca. Trzymałbym się konwencji czyli zwykłego T, a jak już chcesz dać temu dłuższą nazwe to prędziej type, bo tym to jest, typem nie danymi.
2. 6/6 Memory access violation
data Pop()
{
data returnVal;
returnVal = container[objCount - 1];
w ostatniej linijce poleci wyjątek dla container[-1] gdy zrobie pop na pustym stosie. Chyba nie tak to powinno działać, no nie?
3. 5/6 wycieki pamięci w cholere.
Widzę sporo new a nie widzę żadnych delete, czyli masz sporo wycieków pamięci. Nie widać tego jeśli programy są prościutkie i działają krótko, ale gdyby np przeglądarka internetowa gdzieś korzystała z takiej implementacji stosu to bardzo szybko zaczęłaby zużywać o kilka GB ramu za dużo. Dam prosty przykład:
Stack<std::string> s;
for (size_t i = 0; i < 10000; i++)
{
s.Push("dupa");
}
a to wcale nie jest jakiś mocny stress test. Zgadnij ile ramu zjadł ten prosty program? 6 GB :D.
No ale przecież to stos "aż" 10 tys stringów (to aż jest ironiczne :D). A co jeśli będziemy używać niewielkich stosów i je niszczyć, bardzo częsty use case:
for (size_t i = 0; i < 10000000; i++)
{
Stack<std::string> s;
s.Push("dupa");
}
dochodzimy do ostatniej linijki i wielkie zaskoczenie - mimo, że nie używamy już żadnych stosów (wszystkie zwolniliśmy) to program zajmuje 2GB pamięci.
4. 5/6 skorelowane mocno z poprzednim
void Push(data obj)
{
buffer = container;
container = new data[objCount + 1];
czemu dodanie pojedyńczego elementu do stosu wymaga zawsze przepiasania całego stosu od zera? Troche to mało optymalne, no nie? I przede wszystkim alokacji wielgachnych obszarów pamięci dla dużych programów. Puściłem pare stress testów i powiem Ci, że było po szumie wiatraka słychać alokacje pamięci :D
6. 5/6
czemu dodanie jednego elementu wymaga przepisania wszystkich poprzednich? Use case stosu to częste dodawania i sciaganie z końca. Wiesz jak wolno to będzie działać jesli będziemy za każdym razem wszystko kopiować?
5. 5/6
data Pop()
{
data returnVal;
returnVal = container[objCount - 1];
buffer = container;
container = new data[objCount - 1];
wut? sciagniecie elementu ze stosu wymaga zaalokowania tablicy na objCount - 1? Rozumiem, że chcemy strasznie oszczędzać pamięć (pomimo wycieków, które powodują, że tracimy jej jeszcze wiecej), ale po co za każdym razem pomniejszać tablice kosztem przeaalokowania całego stosu. Przy stosie na 10000 elementów po sciagnieciu jednego przealokowujemy 9999, tylko po to żeby zaoczędzić sizeof(element). Alokacja pamięci też trwa, wiesz o tym?
6. 1/6
data returnVal;
returnVal = container[objCount - 1];
czemu nie od razu:
data returnVal = container[objCount - 1];
7. 2/6
if (buffer != nullptr)
{
for (int i = 0; i < objCount; i++)
jesli buffer będzie == nullptr to objCount będzie 0, więc można by sprawdzać czy mamy pusty stos, co byłoby bardziej czytelne. Tak na marginesie to jak mamy pusty stos to objCount == 0, więc pętla i tak sie ani raz nie wykona więc sprawdzenie jest ogólnie bez sensu w tej wersji.
Na razie tyle, jak coś zauważe to jeszcze bd pisał. Generalnie to bardzo kiepska implemenetacja stosu ;/