Stos obiektowy - dobrze napisałem kod?

Napisałem stos obiektowy, chciałem spytać czy jest dobrze. I mam jeden problem do stosu s2 muszę przekopiować zawartość stosu s1. Ale do stosu s2 kopiuje elementy od tyłu, a chcę od przodu aby potem stosem s2 wyświetlało stos s1. Da się ?

#include <iostream>

using namespace std;

class stack
{
public:
    int m_count;
    int max_size;
    int *tablica;

    ////////////////////////////////////////////////////////////////////////

    void init(int max)
    {
        max_size = max;
        m_count = 0;
        tablica = new int [max_size];
    }

    ////////////////////////////////////////////////////////////////////////

    bool empty()
    {
        return m_count == 0;  // !m_count
    }

    ////////////////////////////////////////////////////////////////////////

    bool full()
    {
        return m_count == max_size;
    }

    ////////////////////////////////////////////////////////////////////////

    void destroy()
    {
        while(Pop());
    }

    ////////////////////////////////////////////////////////////////////////

    void Push(int number)
    {
        if(m_count < max_size)
        {
            tablica[m_count++] = number;
        }

    }

    ////////////////////////////////////////////////////////////////////////

    int Top(void)
    {
        if(m_count)
        {
            return tablica[--m_count];
        }

    }

    ////////////////////////////////////////////////////////////////////////

    int Pop()
    {
        if(m_count)
        {
            --m_count;
            return 1;
        }
        else return 0;
    }

};
int main()
{
    int number_user;
    int count_number;

    cout << "Podaj ilosc liczb ktore chcesz wlozyc na stos: ";
    cin >> count_number;

    stack s1;
    stack s2;
    s2.init(10);
    s1.init(count_number);

    cout << "\nPodawaj kolejne liczby stosu\n\n";
    for(int i = 0; i < count_number; ++i)
    {
        cin >> number_user;
        s1.Push(number_user);
    }

    for(int i = 0; i < count_number; ++i)
    {
       s2.Push(s1.Top());
    }

    cout << s2.Top();



    return 0;

}
 

Z tym top i pop to coś namieszałeś.

To pop powinno zrzucić element z wierzchołka stosu, a w twoim kodzie to zwraca 1 albo 0.
top, a poprawniej to byłoby peek powinno zwracać element z wierzchołka stosu nie usuwając go.

I dlaczego nie zrobisz normalnego konstruktora tylko jakieś init ?

Miałem polecenie aby był bez konstruktorów. Co do zwracania 1 i 0 w topie, to dlatego bo w funkcji destroy mam pętle

while(Pop());

Ale jaki sens ma w takim razie ta metoda pop ?

Równie dobrze mogłeś w metodzie destroy zamiast :

 while(Pop()); 

Zrobić:

 m_count=0;

Teraz masz nieścisłość w nazewnictwie, bo twoje metody nie robią tego na co wskazuje ich nazwa.

Zdajesz sobie sprawę, że to Twoje Destroy tak naprawdę nic nie robi? :P

Tak, kierowałem się stroną lo Tarnów. W funkcji destroy do kolejnych elementów tablicy wpisywać 0 ? Nie mogę używać destruktora.

Musisz zwalniać pamięć.

Destroy ma czyścić pamięć. To nie wiem czy zwalniać pamięć czy wpisywać 0 do poszczególnych elementów tablicy ?

Zwalniać pamięć powinieneś w destruktorze, a w konstruktorze przydzielać, bądź ustawiać na NULL zmienną tablica.

To zwalniać przez destroy czy wpisywać zera do poszczególnych elementów tablicy ? Ten program ma być bez konstruktorów i destruktorów.

konstruktor i destruktor będzie zawsze, a jeśli jest coś alokowane dynamicznie, to trzeba napisać własny - to jest zasada, którą należy stosować, inaczej będą wycieki pamięci. O ile w systemie z ochroną pamięci, ubijesz program i po krzyku (co najwyżej system podczas działania programu zacznie zwalniać), to czasami ze względów wydajnościowych używasz systemów bez ochrony pamięci, bądź ze szczątkowa i wtedy zakończenie pracy takiego programu nie zwróci nam pamięci! A właśnie programowanie obiektowe powstało między innymi dlatego (po za wszelkimi projektowymi rzeczami), aby ograniczyć wycieki, aby moc to zautomatyzować.

Czy teraz jest dobrze ?:

#include <iostream>

using namespace std;

struct stack
{
public:
    int m_count;
    int max_size;
    int *tablica;

    ////////////////////////////////////////////////////////////////////////

    void init(int max)
    {
        max_size = max;
        m_count = 0;
        tablica = new int [max_size];
    }

    ////////////////////////////////////////////////////////////////////////

    bool empty()
    {
        return m_count == 0;  // !m_count
    }

    ////////////////////////////////////////////////////////////////////////

    bool full()
    {
        return m_count == max_size;
    }

    ////////////////////////////////////////////////////////////////////////

    void destroy()
    {
        delete [] tablica ;
    }

    ////////////////////////////////////////////////////////////////////////

    void Push(int number)
    {
        if(m_count < max_size)
        {
            tablica[m_count++] = number;
        }

    }

    ////////////////////////////////////////////////////////////////////////

    int Top(void)
    {
        if(m_count)
        {
            return tablica[--m_count];
        }

    }

    ////////////////////////////////////////////////////////////////////////

    int Pop()
    {
        if(m_count)
        {
            --m_count;
        }
    }

};
int main()
{
    int number_user;
    int count_number;

    cout << "Podaj ilosc liczb ktore chcesz wlozyc na stos: ";
    cin >> count_number;

    stack s1;
    stack s2;

    s1.init(count_number);
    s2.init(count_number);

    cout << "\nPodawaj kolejne liczby stosu\n\n";
    for(int i = 0; i < count_number; ++i)
    {
        cin >> number_user;
        s1.Push(number_user);
    }

    for(int i = 0; i < count_number; ++i)
    {
       s2.Push(s1.Top());
    }

    cout << s2.Top();



    return 0;

}
 

Nie, bo metody pop i top nadal nie robią tego co powinny(i na co wskazuje nazwa).

top - odczytanie z wierzchołka stosu liczby (tzn. liczby ostatnio położonej)

Wykonuję taką czynność, czemu nie jest ona dobra ?

top powinna odczytywać, ale nie zdejmować, pop powinna odczytywać i zdejmować. Dodatkowo nie masz obsłużonego błędu przepełnienia i próby zdjęcia z pustego stosu.

To w topie dać jeszcze na końcu instrukcję :

 ++m_count

?

tablica[m_count-1];

Nie myślisz.
Po co w ogóle w tym top masz m_count--?

Bo m_count pokazuje na pusty element stosu, --m_cout pokazuje na ostatni element stosu.

`dodanie znaczników ``` - @furious programming

No to powinno być tak jak pokazał @twonek.

 m_count-1 

faktycznie, dzięki. poza tym program jest ok ?

nowicjusz35 napisał(a):

faktycznie, dzięki. poza tym program jest ok ?

Jeśli poprawiłeś te błedy, o których była mowa, to jest na to szansa. Masz nieprzewidywalne działania w kilku miejscach - np zwracana niewiadoma wartość.

#include <iostream>
 
using namespace std;
 
struct stack
{
public:
    int m_count;
    int max_size;
    int *tablica;
 
    ////////////////////////////////////////////////////////////////////////
 
    void init(int max)
    {
        max_size = max;
        m_count = 0;
        tablica = new int [max_size];
    }
 
    ////////////////////////////////////////////////////////////////////////
 
    bool empty()
    {
        return m_count == 0;  // !m_count
    }
 
    ////////////////////////////////////////////////////////////////////////
 
    bool full()
    {
        return m_count == max_size;
    }
 
    ////////////////////////////////////////////////////////////////////////
 
    void destroy()
    {
        delete [] tablica ;
    }
 
    ////////////////////////////////////////////////////////////////////////
 
    void Push(int number)
    {
        if(m_count < max_size)
        {
            tablica[m_count++] = number;
        }
 
    }
 
    ////////////////////////////////////////////////////////////////////////
 
    int Top(void)
    {
        if(m_count)
        {
            return tablica[m_count - 1];
        }
 
    }
 
    ////////////////////////////////////////////////////////////////////////
 
    int Pop()
    {
        if(m_count)
        {
            --m_count;
        }
    }
 
};
int main()
{
    int number_user;
    int count_number;
 
    cout << "Podaj ilosc liczb ktore chcesz wlozyc na stos: ";
    cin >> count_number;
 
    stack s1;
    stack s2;
 
    s1.init(count_number);
    s2.init(count_number);
 
    cout << "\nPodawaj kolejne liczby stosu\n\n";
    for(int i = 0; i < count_number; ++i)
    {
        cin >> number_user;
        s1.Push(number_user);
    }
 
    for(int i = 0; i < count_number; ++i)
    {
       s2.Push(s1.Top());
    }
 
    cout << s2.Top();
 
 
 
    return 0;
 
} 

Pop ma tylko zdejmować, tak mam w zadaniu. Coś jeszcze poprawić?

pop powinien też zwracać element z wierzchołka.