Skip to main content
added 26 characters in body
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

Ok, a few comments:

  1. If you are going to support a stack with no capacity, I don't know why you need to allocate memory at all. You could simply have the pointer as NULLNULL.

  2. Using coutcout for error handling is not appropriate. You could either give "undefined behaviour" however may I suggest you throw an exception. You can handle your exceptions and use coutcout in your exception handler.

  3. Implement a deep copy constructor and implement swap()swap(). Then implement assignment in terms of both of these. To swap you would do:

    void MyStaticIntStack::swap( MyStaticIntStack & other ) { // swap each member }

     void MyStaticIntStack::swap( MyStaticIntStack & other )
     {
         // swap each member
     }
    

If you don't like using std::swap, write your own. I am not sure you have to avoid all STL, you probably cannot use STL containers for this because the exercise is to write your own container, but utilities like swap may be permitted (plus you can throw std exceptions).

I don't see why -1 shouldn't be a valid number in your stack. If someone does not know whether your stack is empty and tries a peek() from code, they would get -1 (plus some coutcout that they cannot handle). Perhaps have a method

bool empty() const;

which tells you if the stack is empty or not. You could also have size() and capacity() access methods.

You might want to be able to resize the capacity of your stack. If you find that difficult, have a private constructor that takes a new capacity plus a reference to an existing stack. That constructor can create the data with the relevant size and copy the data into it.

Your internal method would call this constructor to create a new bigger capacity stack, then invoke the swap()swap() method. The temporary one would now disappear. (Do not create it with new).

On a style issue:

  1. Initialize your members in the constructor initialization list, not the body of your constructor.
  2. Do not use this-> all over the code.

Ok, a few comments:

  1. If you are going to support a stack with no capacity, I don't know why you need to allocate memory at all. You could simply have the pointer as NULL.

  2. Using cout for error handling is not appropriate. You could either give "undefined behaviour" however may I suggest you throw an exception. You can handle your exceptions and use cout in your exception handler.

  3. Implement a deep copy constructor and implement swap(). Then implement assignment in terms of both of these. To swap you would do:

    void MyStaticIntStack::swap( MyStaticIntStack & other ) { // swap each member }

If you don't like using std::swap, write your own. I am not sure you have to avoid all STL, you probably cannot use STL containers for this because the exercise is to write your own container, but utilities like swap may be permitted (plus you can throw std exceptions).

I don't see why -1 shouldn't be a valid number in your stack. If someone does not know whether your stack is empty and tries a peek() from code, they would get -1 (plus some cout that they cannot handle). Perhaps have a method

bool empty() const;

which tells you if the stack is empty or not. You could also have size() and capacity() access methods.

You might want to be able to resize the capacity of your stack. If you find that difficult, have a private constructor that takes a new capacity plus a reference to an existing stack. That constructor can create the data with the relevant size and copy the data into it.

Your internal method would call this constructor to create a new bigger capacity stack, then invoke the swap() method. The temporary one would now disappear. (Do not create it with new).

On a style issue:

  1. Initialize your members in the constructor initialization list, not the body of your constructor
  2. Do not use this-> all over the code.

Ok, a few comments:

  1. If you are going to support a stack with no capacity, I don't know why you need to allocate memory at all. You could simply have the pointer as NULL.

  2. Using cout for error handling is not appropriate. You could either give "undefined behaviour" however may I suggest you throw an exception. You can handle your exceptions and use cout in your exception handler.

  3. Implement a deep copy constructor and implement swap(). Then implement assignment in terms of both of these. To swap you would do:

     void MyStaticIntStack::swap( MyStaticIntStack & other )
     {
         // swap each member
     }
    

If you don't like using std::swap, write your own. I am not sure you have to avoid all STL, you probably cannot use STL containers for this because the exercise is to write your own container, but utilities like swap may be permitted (plus you can throw std exceptions).

I don't see why -1 shouldn't be a valid number in your stack. If someone does not know whether your stack is empty and tries a peek() from code, they would get -1 (plus some cout that they cannot handle). Perhaps have a method

bool empty() const;

which tells you if the stack is empty or not. You could also have size() and capacity() access methods.

You might want to be able to resize the capacity of your stack. If you find that difficult, have a private constructor that takes a new capacity plus a reference to an existing stack. That constructor can create the data with the relevant size and copy the data into it.

Your internal method would call this constructor to create a new bigger capacity stack, then invoke the swap() method. The temporary one would now disappear. (Do not create it with new).

On a style issue:

  1. Initialize your members in the constructor initialization list, not the body of your constructor.
  2. Do not use this-> all over the code.
Source Link
CashCow
  • 532
  • 2
  • 4

Ok, a few comments:

  1. If you are going to support a stack with no capacity, I don't know why you need to allocate memory at all. You could simply have the pointer as NULL.

  2. Using cout for error handling is not appropriate. You could either give "undefined behaviour" however may I suggest you throw an exception. You can handle your exceptions and use cout in your exception handler.

  3. Implement a deep copy constructor and implement swap(). Then implement assignment in terms of both of these. To swap you would do:

    void MyStaticIntStack::swap( MyStaticIntStack & other ) { // swap each member }

If you don't like using std::swap, write your own. I am not sure you have to avoid all STL, you probably cannot use STL containers for this because the exercise is to write your own container, but utilities like swap may be permitted (plus you can throw std exceptions).

I don't see why -1 shouldn't be a valid number in your stack. If someone does not know whether your stack is empty and tries a peek() from code, they would get -1 (plus some cout that they cannot handle). Perhaps have a method

bool empty() const;

which tells you if the stack is empty or not. You could also have size() and capacity() access methods.

You might want to be able to resize the capacity of your stack. If you find that difficult, have a private constructor that takes a new capacity plus a reference to an existing stack. That constructor can create the data with the relevant size and copy the data into it.

Your internal method would call this constructor to create a new bigger capacity stack, then invoke the swap() method. The temporary one would now disappear. (Do not create it with new).

On a style issue:

  1. Initialize your members in the constructor initialization list, not the body of your constructor
  2. Do not use this-> all over the code.