Trouble with dynamic array

[Xilonian]

Im writing a class to try out dynamic memory allocation. The class is for a dynamic array of integers and has 3 data members. A pointer to the first array element, an integer for array size and one for capacity. The << operator is also overloaded to allow printing of the array. When i declare my array then create a vector object and try to print it segmentation faults. I have other methods but only the ones being tested are posted here. The rest of them are never called (commented out). The vector array should be initialized to null in the constructor hence the 0.

//Header
#ifndef VECTOR_H
#define VECTOR_H
#include <iostream>
using namespace std;

class Vector
{
	public:
		Vector();
                friend ostream& operator<<(ostream&, Vector);
        private:
		int * vectorArray;
		int vectorSize;
		int vectorCapacity;
};

#endif 

//vector.cpp file
Vector::Vector()
{
	vectorArray = new int{0};
	vectorSize=0;
	vectorCapacity=0;
}

ostream& operator<<(ostream& leftOp, const Vector rightOp)
{
	for(int i=0; i<rightOp.vectorSize; i++)
	{
		leftOp<<rightOp.vectorArray[i];
		leftOp<<", ";
	}
	return leftOp;
}

//main program to test
#include <iostream>
#include "vector.h"

using std::cout;
using std::endl;

int main()
   {
   cout << "Testing default constructor\n\n";

   Vector v1;
   
   cout << "v1: " << v1 << endl;   

   return 0;
   }

[Ispil]

Look at the implementation file, on line 4. There should be brackets, not braces.

[Xilonian]

It's still giving me a seg fault no matter how i declare the array. I can call the constructor fine and it will go through but printing it using the overloaded << seg faults.

I check the size and instead of being 0 it shows as 4196736 when i use the overloaded << using this code:

ostream& operator<<(ostream& leftOp, const Vector rightOp)
{
      leftOp<<rightOp.vectorSize;
      return leftOp;
}

[D80DSL3A]

The problem may be with code you aren't showing.

Xilonian wrote:

I have other methods but only the ones being tested are posted here. The rest of them are never called

Not everything gets called explicitly.
For example, you are passing rightOp by value to operator <<, so a local copy is created, and then it's destroyed. The copy constructor and destructor are invoked "silently".
Did write you own version of them? Perhaps you forgot to assign a value to vectrorSize in the copy constructor, leaving vectorSize uninitialized.

The code you have posted looks (sort of) OK.

[cire]

Your operator<< implementation takes its Vector parameter by value, which means it will take a copy of the argument fed to it. However, you don't explicitly supply a copy constructor and the defaulted one is not adequate.

If you were to change operator<< to take it's Vector parameter by reference, your problem would go away since there would be no copy construction involved. (This is assuming you've defined a destructor that frees the memory. If you haven't, the only issue would be a memory leak.)

[Xilonian]

here is my copy constructor and deconstructor:
Fixing the code up as i figured out a few things that were wrong

Vector::~Vector()
{
delete [] vectorArray;
}

Vector::Vector(const Vector& vectorToCopy)
{
        for(int i=0; i<vectorToCopy.vectorSize; i++)
        {
        this->vectorArray[i]=vectorToCopy.vectorArray[i];
        }
}

I was assuming the copy constructor simply did the copy and did not have to modify the
size or capacity

[cire]

Your copy constructor is incorrect. It doesn't initialize vectorArray, vectorSize or vectorCapacity to meaningful values, and it copies to the uninitialized value (read:random address) held by vectorArray.

[Xilonian]

Modified copy constructor:

Vector::Vector(const Vector& vectorToCopy)
{
        vectorSize=vectorToCopy.vectorSize;
        vectorCapacity=vectorToCopy.vectorCapacity;
        vectorArray=new int[vectorCapacity];
        assert (vectorArray!=0);
        for(int i=0; i<vectorCapacity;i++)
        {
                vectorArray[i]=vectorToCopy.vectorArray[i];
        }

}

[cire]

You also have a problem here:

Vector::Vector()
{
	vectorArray = new int{0};
	vectorSize=0;
	vectorCapacity=0;
}

Your destructor uses delete [] vectorArray, but vectorArray is not allocated with new[] in the default constructor. new should be paired with delete, and new[] should be paired with delete[].

I'd probably write it like so:
Vector::Vector() : vectorArray(nullptr), vectorSize(0), vectorCapacity(0) {}

Note that deleting a nullptr is a perfectly safe thing to do.

I might write the copy constructor like so:

Vector::Vector(const Vector& v)
    : vectorArray(new int [v.vectorCapacity]), 
      vectorSize(v.vectorSize), 
      vectorCapacity(v.vectorCapacity)
{
    for ( unsigned i=0; i<vectorSize; ++i )
        vectorArray[i] = v.vectorArray[i] ;
}