Pointers inside class

[Nis]

Do you think I added the deconstructor, operator = and copy constructor correctly?

#include <iostream>
#include <string>
using namespace std;

class c
{
	int *p;
	int s;
public:
	c()
	{p=0; s=0;}
	void set (int v)
	{
		if (p!=0) delete []p;
		s=v;
		p=new int [s];
		for (int i=0; i<s; i++)
			cout<<p[i]<<endl;}
	~c() 
	{delete p;
	} //memory leak problem solved

	void operator = (c &o)
	{
		if (p!=0) delete p;
		p=new int;
		*p=*(o.p); //shallow copy problem solved
		
	
	}

	c (c &x)
	{
		p=new int;
		*p=*(x.p);
		
	}


	void print ()
	{
		cout<<s;
		if (s!=0)
			for (int i=0;i<s; i++)
				cout<<p[i]<<endl;
	
	}
};


void main ()
{
	c o;
	o.set(3);
}

[shadowCODE]

p!=0

Zero sounds like NULL but is not.
Don't compare pointer and integer.

You could simply say if(!p) or explicityly as if(p == NULL) or if(p == nullptr).

[Nis]

ok thanks!

besides that, are these correct?
~c()
{delete p;
} //memory leak problem solved

void operator = (c &o)
{
if (p!=0) delete p;
p=new int;
*p=*(o.p); //shallow copy problem solved
}

c (c &x)
{
p=new int;
*p=*(x.p);
}

do I have to do things like delete []p since I have an array or can I keep it as it is?

[Peter87]

p!=0 works too. The other methods mentioned by shadowCODE also works (NULL is often defined as 0).

Many of the null and zero checks are unnecessary. Using delete on a null pointer will do nothing and is perfectly safe. Also in the print() function you don't have to check if s is not zero because the loop condition already takes care of that.

When deleting an array you should use delete[].

Your copy constructor and copy assignment operator should use const qualifiers on the parameters (const c& o) so they can be used with temporary objects and variables marked with const:

c var1 = c(); // error because constructor does not accept a const argument
const c var2;
var1 = var2; // error because operator= does not accept a const argument 

You also have a problem because p=new int; will allocate one int (not an array) and *p=*(x.p); will assign to that one int. To allocate an array you will have to pass the size inside [ ] like you do in the set function. To copy all values from the other array you should use a loop.

[Nis]

I've changed it to look like this, but I'm getting an error at *p[i]=*(x.p[i]); saying it must be a pointer,,, but it is. I am very confused :/

class c
{
	int *p;
	int s;
public:
	c()
	{p=0; s=0;}
	void set (int v)
	{
		if (p!=0) delete []p;
		s=v;
		p=new int [s];
		for (int i=0; i<s; i++)
			cout<<p[i]<<endl;}
	~c() 
	{delete []p;
	} //memory leak problem solved

	void operator = (const c &o)
	{
		if (p!=0) delete []p;
		p=new int [s];
		*p=*(o.p); //shallow copy problem solved
		
	
	}

	c (const c &x)
	{
		p=new int[s];
		for (int i=0; i<s; i++)
		*p[i]=*(x.p[i]);
		
	}


	void print ()
	{
		cout<<s;
		if (s!=0)
			for (int i=0;i<s; i++)
				cout<<p[i]<<endl;
	
	}
};

[Nis]

NO now I changed it to this and am not getting an error, is this correct now?

class c
{
	int *p;
	int s;
public:
	c()
	{p=0; s=0;}
	void set (int v)
	{
		if (p!=0) delete []p;
		s=v;
		p=new int [s];
		for (int i=0; i<s; i++)
			cout<<p[i]<<endl;}
	~c() 
	{delete []p;
	} //memory leak problem solved

	void operator = (const c o[])
	{
		if (p!=0) delete []p;
		p=new int [s];
		for (int i=0; i<s; i++)
		*p=*(o[i].p); //shallow copy problem solved
		
	
	}

	c (const c x[])
	{
		p=new int[s];
		for (int i=0; i<s; i++)
		*p=*(x[i].p);
		
	}


	void print ()
	{
		cout<<s;
		if (s!=0)
			for (int i=0;i<s; i++)
				cout<<p[i]<<endl;
	
	}
};

But I get an assertion error when I put this in the main:
void main ()
{
c o;
o.set(3);
o.print();
c o1;
o1.set(2);
o1.print();
o=o1;
o.print();
o1.print();
}

[Peter87]

I've changed it to look like this, but I'm getting an error at *p[i]=*(x.p[i]); saying it must be a pointer,,,

It gave you an error because p[i] and x.p[i] are not pointers, they are the integers at position i in the arrays. Using the deference operator * on an integer doesn't make sense so it gives you an error.

[Nis]

^I changed that and rewrote the code in the post above yours. I double posted heh

I'm still getting an error though :/

[Peter87]

I saw that but I ignored it because it only made things worse. Try to understand what you are doing instead of just guessing.

[Nis]

:(

[Nis]

Please someone tell me how to correct these 2 for arrays i am so desperate. I never took Data structures 1. They moved me directly to 2 because apparently i am smart but I dont understand anything

void operator = (const c &o)
{
if (p!=0) delete []p;
p=new int [s];
for (int i=0; i<s; i++)
*p=*(o.p); //shallow copy problem solved
}

c (const c &x)
{
p=new int[s];
for (int i=0; i<s; i++)
*p=*(x.p);

}

E: I just want to know how to place the [] for the loops it isnt working :(

[Nis]

I think I've got it!! Is this right now!!

class c
{
	int *p;
	int s;
public:
	c()
	{p=0; s=0;}
	void set (int v)
	{
		if (p!=0) delete []p;
		s=v;
		p=new int [s];
		for (int i=0; i<s; i++)
			cin>>p[i];
	}

	~c() 
	{
		delete []p;
	} //memory leak problem solved

	void operator = (const c &o)
	{
		if (p!=0) delete []p;
		p=new int [s];
		for (int i=0; i<s; i++)
		{
			*(p+i)=*((o.p)+i); //shallow copy problem solved
		}
	}

	c (const c &x)
	{
		p=new int[s];
		for (int i=0; i<s; i++)
		{
			*(p+i)=*((x.p)+i);
		}	
	} //shallow copy during function call solved

	void print ()
	{
		cout<<"Number of elements: "<<s<<endl;
		cout<<"Elements include: ";
		if (s!=0){
			for (int i=0;i<s; i++)
				cout<<p[i]<<'\t';}
		
	}
};

[Peter87]

It looks fine, except that you also need to copy s.

[ne555]

you should check for self-assignment.

[Nis]

Why should I copy s? It's all declared in the same class :/

Self assignment??

[Peter87]

Why should I copy s? It's all declared in the same class :/

Think about what will happen if you assign an object to another object whose value of s is different.

Self assignment??

Self assignment is when you assign an object to itself.

c obj;
obj = obj;

Another thing I noticed is that you never write any values to the array elements. They are not even initialized.