Understanding pointers and bad memory allocation?

Question

Disclaimer: I am very very very new to C++. I took one class two years ago, and I'm taking another class now, so trying to get back into the mindset is tough. That being said, I know my code is extremely bad; I would like help and constructive criticism, so please avoid outright bashing, because I know I don't really know what I'm doing.

Objective: I'm writing a program that takes in two user-input sets of the form {1 2 3} (any size), and performs some operations on them. But I'm having trouble simply reading them in. Here is a quick snippet of the header file that shows what functions I'll be asking about:

class Set{

  public:

    Set();                  //Constructor for empty set.
    Set(int element);       //Constructor for set of one element
    ~Set();

    unsigned int getSize() const;                          
    int operator[](const int e) const;  

    friend ostream& operator<<(ostream& oss, const Set& output);   
    friend istream& operator>>(istream& ss, Set& target);         

  private:

    int size;
    int *elements[];        
    void resize(unsigned int new_size);
};

There's more, but I'm just showing you all what's necessary.

For the assignment, we create a class called Set, but the set must be stored as an array. We have an array of pointers for each Set (i.e. the Set class comes with int *elements[]). We need to overload the ">>" operator to convert the user-input to a Set. The user inputs two sets. We are assuming the user is smart enough to type it in in the exact form {1 2 3 4}, but it can be any size. Here is my overloaded >>.

istream& operator>>(istream& ss, Set& target){ 

  //The constructor Set target; creates an empty Set with array size=0.

  char c;      

  if (ss.peek()=='}') ss >> c; 
  if (ss.peek()==EOF) ss >> c;

  ss >> c;     //First we read in the '{' character.
  int num;

  while (ss.peek()!='}'){
    ss >> num;
    target.resize(target.getSize()+1); 
    *target.elements[target.getSize()-1]=num;
  };
  return ss;
};

getSize() is simply a function that returns the value "size." The goal of the above snippet is to create an empty Set, then for every int that is between "{" and "}", we resize the array by 1, then put that int into the next element of the array. Also, streams are still as abstract to me as pointers, so that's why this seems a little brute-force.

The problem I am having is with resize. I want--obviously--for resize to change the size, but it does not allow this. What I am trying to accomplish is for resize to change the called Set's *elements pointer to point to a new array with new_size number of elements. Then I want to deallocate that pointer to avoid memory leaks.

void Set::resize(unsigned int new_size){

  if (size!=new_size){
      int *newelements = new int[new_size];

      for (int i=0; i<new_size || i<size; i++){
          newelements[i] = *elements[i]; 
      };

      *elements = newelements;
      delete newelements;

      if (size>new_size) size=new_size;
   };

};

One thing I know is wrong: when I call newset.resize in the definition for ">>", the

newelements[i] = *elements[i];

line is trying to assign something that doesn't exist yet since "num" hasn't yet been assigned to *newset.elements[i]. However, if I try switching the order of these in ">>" so that it goes like this instead...

*target.elements[target.getSize()]=num;
target.resize(target.getSize()+1); 

...this shouldn't make sense because the size of target's array is 0, so I can't say target[0]=num;

Another thing I know is wrong: Something must be wrong with the way I'm using the pointers because if I try to say size=new_size in the resize definition, I get the "EXC_BAD_ACCESS" error. Actually, if I run this as is, I get the same error message.

Additionally, in the assignment, we are told that "resize should only update size if new_size is strictly smaller than size." I don't understand how this can ever work. If we have to start with an initialized array of size=0, then how can we ever make it bigger?

I hope I have included enough to get a little help on this. I appreciate anything that would help me understand pointers and how to use them. Thank you so much!

Answer 1

I'm not sure if I picked up everything, but scanning through the code:

1. The declaration int *elements[]; is illegal in C++ although some compilers allow it as an extension. You must specify the array size. If I compile with gcc, it tells me:

   warning: ISO C++ forbids zero-size array 'elements' [-Wpedantic]

   int *elements[];

2. The line if (size!=new_size) is true always unless size does not equal new_size. Most likely you want to resize the array only when new_size is greater than size.

3. i<new_size || i<size should probably be just i<size.

4. In newelements[i] = *elements[i];, the *elements[i] is equivalent to *(elements[i]) and not (*elements)[i], are you sure that is what you want?

5.

*elements = newelements;
delete newelements;

The first line makes the first element (a pointer) of elements to point at the newly allocated memory. The second line then deletes that memory, so now the first element of elements is now pointing at deallocated memory. That is not what you want. Remember, when you assign a pointer to a pointer, it makes both pointers point at the same memory. What you will need to do is delete the old memory and then assign the pointer to point at the newly allocated memory.

6. Also remember that delete [] is different from delete. You will need to use delete [] when deleting the block of memory.

And hopefully this is all for learning purposes. In real C++ code, you would be using std::vector instead.