Wat is er verkeerd aan mijn push method

Hoi,

Ik ben uitgedaagd om mijn eigen stack class te schrijven om te oefenen met pointers.
Later "moet" ik dit herschrijven en smart pointers gebruiken.

Voor zover heb ik dit :

main.cpp

#include <iostream>
#include <string>
#include "stack.h"


int main() {
    Stack s;
    std::string line;
    while (true) {
        std::cout << "> ";
        std::getline(std::cin, line);
        if (line == "exit") break;
        else if (line == "pop") std::cout << "Popped " << s.pop() << "\n";
        else if (line == "top") std::cout << "Top value: " << s.top() << "\n";
        else if (line.compare(0, 4, "push") == 0) {
            int val = std::stoi(line.substr(5));
            s.push(val);
        } else if (line == "print") {
            Stack copy = s;
            while (copy.size() > 0) {
                std::cout << copy.pop() << " ";
            }
            std::cout << "\n";
        } else {
            std::cout << "Commands: push <n>, pop, top, exit\n";
        }
    
    }
}

stack.h

#pragma once

class Stack {
    public:
        // Creates an empty stack with capacity 4
        Stack();
        
        
        // Creates a copy of your stack
        Stack(const Stack& o);
            
         
        // Assignment operator
        Stack& operator=(const Stack& o);

         
        
        // Destructor
        ~Stack();

              
        // We'll ignore move constructor and move assignment.
                  
       
        // Adds a new value to the stack
        void push(int value);

       
        
        // Checks the value at the top of the stack
        int top();

        // Removes the value at the top of the stack (returning it)
        int pop();
        
        // Returns the number of elements in the stack
        int size();
    
    private: 
        int* buffer;
        int capacity;
        int number_of_items;
    
     
}; 

stack.cpp

#include "stack.h"
#include <memory>

Stack::Stack() {
    capacity = 4;
    buffer = new int[capacity]; 
    number_of_items = 0;
}

Stack::Stack(const Stack& o){
    capacity = o.capacity; 
    number_of_items = o.number_of_items; 
    buffer = new int[capacity];
    for (int i {}; i < number_of_items; i++) {
        buffer[i] = o.buffer[i];  
     }  
}


Stack& Stack::operator =(const Stack& o) {
    capacity = o.capacity;   
    number_of_items = o.number_of_items;
    delete[] buffer;  
    buffer = new int[capacity];
    for (int i {}; i < number_of_items; i++) {
        buffer[i] = o.buffer[i];  
    }
    
    return *this; 
}

Stack::~Stack(){
    delete[] buffer;  
}

void Stack::push(int value){
    if (number_of_items <= capacity) {
        ++number_of_items; 
        buffer[number_of_items] = value;
    } else {
        capacity = capacity * 2 ; 
        int* new_buffer = new int[capacity];
        for (int i {}; i < number_of_items; i++) {
            new_buffer[i] = buffer[i];  
        }; 
        delete[] buffer;
        buffer = new int[capacity]; 
        buffer = new_buffer;  
    }
}

int Stack::top()  {
    return buffer[number_of_items -1]; 
}


int Stack::pop() {
    return buffer[number_of_items - 1];
    --number_of_items;  
}


int Stack::size() {
    return number_of_items; 
}


maar als ik doe "push 4 " en daarna print dan wordt er een doorlopende loop gestart waar alleen maar nullen wordt geprint.

Nu vraag ik me af of iemand me kan helpen om uit te vinden waar ik verkeerd gedacht heb of waar de code verkeerd is

In de pop-methode verlaag je number_of_items niet correct. De volgorde van de instructies is onjuist: je moet eerst de waarde retourneren en daarna number_of_items verlagen. Momenteel wordt de waarde geretourneerd voordat de grootte is aangepast, wat het correcte gedrag van de stack verhindert. (De aanroep van return beëindigt de functie.).


Controleer de indexen. Een array begint bij 0.

En controleer hoe je het geheugenbeheer uitbreidt...

(Je hebt een RAM-geheugenlek omdat er een dubbele toewijzing plaatsvindt.)

Dank je

Ik wijs een nieuwe buffer aan omdat ik er net voor de buffer "verwijder" met delete[] buffer . Dus ik dacht dat ik de buffer dan weer opnieuw moest initialiseren.

En wat bedoel je hiermee precies :

Controleer de indexen. Een array begint bij 0.

oke

ik heb veranderd wat je me gevraagd hebt en het lijkt iets beter

nu lijkt de push method zo :

void Stack::push(int value){
    if (number_of_items <= capacity) {
        ++number_of_items; 
        buffer[number_of_items] = value;
    } else {
        capacity = capacity * 2 ; 
        int* new_buffer = new int[capacity];
        for (int i {}; i < number_of_items; i++) {
            new_buffer[i] = buffer[i];  
        }; 
        delete[] buffer;
        buffer = new_buffer;
        buffer[number_of_items] = value; 
    }
}

maar ik zie nog dit :

./stack-cli 
> push 5
> print
0 

waar ik verwachte een 5 te zien.
Zie jij nog een fout ?

This writes outside your array...
The last element is number_of_items-1

Misschien krijg je ideeën als je dat bestudeert?

Dank u wel allemaal.

Ik heb het denk ik opgelost op deze manier : https://github.com/RoelofWobben/stack_normal_pointers/blob/main/stack.cpp

Als dit goed is, dan eens kijken of ik het kan herschrijven zodat smart pointers gebruikt worden.

De link werkt niet.

sorry, nu wel
Was vergeten dat de repo private was.
Heb het even op public gezet.

Klinkt goed.

blij om te horen dat er geen rare zaken te vinden zijn.
Denk het "ombouwen" om smart pointers te gebruiken , zou denk ik niet zoveel tijd kosten als dit schrijven.

pfff, ombouwen tot smart pointers is toch moeilijker dan ik dacht

ik heb nu dit :

Stack::Stack() {
    capacity = 4;
    std::unique_ptr<int*[]> buffer; 
    number_of_items = 0;
}

Stack::Stack(const Stack& o) : capacity(o.capacity), number_of_items(o.number_of_items),
      buffer(std::make_unique<int[]>(o.capacity)) 
{
    for (int i = 0; i < number_of_items; ++i) {
        buffer[i] = o.buffer[i]; // deep copy elements
    }
}

en ik krijg deze compile error :

no suitable conversion function from "std::__detail::__unique_ptr_array_t<int []>" (aka "std::unique_ptr<int [], std::default_delete<int []>>") to "int *" exists

mag ik hints hoe dit op te lossen ?

Alles opgelost.

ik hoop dat de code goed is : GitHub - RoelofWobben/stack_normal_pointers

Als je van plan bent dit op kleine microcontrollers of een Arduino te gebruiken, kun je best je geheugentoewijzingsstrategie verfijnen, want je RAM zal snel opraken.

int new_capacity = capacity * 2;

ben ik met je eens. ik heb hier voor 2 gekozen omdat de challenge dit wilde.

Alles opgelost.

1 Like

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.