Using new and delete

why doesn't this work? (Error: corrupt data)

code:
1
2
3
4
5
6
7
8
9
10
11
12
13
Cow & Cow::operator=(const Cow & c)
{
	int len = 1 + strlen(c.name);
	int i = 0;
	for(;i<19 && c.name[i];i++)
		name[i] = c.name[i];
	name[i] = '\0';
	delete[] hobby;
	hobby = new char[strlen(c.hobby) + 1];
	strcpy(hobby, c.hobby);
	weight = c.weight;
	return *this;
}
closed account (Dy7SLyTq)
remove && c.name[i] on line 6 and you need to declare hobby and weight
The code is part of a program... This is the class members,
// char name[20];
// char * hobby;
// double weight;
I use new in my constructor, I don't see why this doesn't work!
Is there any reason why name & hobby are an array of char and not a std::string?

That would make things a lot easier wouldn't it?

Why do you have a copy assignment operator anyway? It does not seem to do anything different to the implicit default one.
The default would just copy the address instead of copying the contents... And it is a programming exercise, can't use string, that would be cheating.
The only obvious problem I see with your operator= is that it will fail miserably for self assignment. Another caveat is that it isn't exception safe.

I would expect something more like:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
Cow& Cow::operator=(const Cow& c)
{
    using std::strlen ;

    if ( &c != this )
    {
        if ( strlen(hobby) < strlen(c.hobby) )      // attempt memory allocation before making any changes.
        {
            char * mem ;
            mem = new char[strlen(c.hobby)+1] ;
            delete [] hobby ;
            hobby = mem ;            
        }

        std::copy(c.name, c.name+strlen(c.name)+1, name) ;
        std::copy(c.hobby, c.hobby+strlen(c.hobby)+1, hobby) ;
    }

    return *this ;
}


But I suspect your problem lies elsewhere.
Last edited on
Yep, didn't really solve the problem but thx, this is the class decleration:
1
2
3
4
5
6
7
8
9
10
11
12
13
class Cow {
private:
	char name[20];
	char * hobby;
	double weight;
public:
	Cow();
	Cow(const char * nm, const char * ho, double wt);
	Cow(const Cow & c);
	~Cow();
	Cow & operator=(const Cow & c);
	void ShowCow() const;
};
I don't see a problem with this ctor. The problem must be elsewhere.
anything wrong with this code?:
1
2
3
4
5
6
7
8
9
10
11
12
int main()
{
	Cow var;
	Cow var2("cow", "running", 393.8);
	var.ShowCow();
	var2.ShowCow();
	var = var2;
	var.ShowCow();

	std::cin.get();
	return 0;
}
Last edited on
it may be that the constructors that are wrong...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
Cow::Cow(const char * nm, const char * ho, double wt)
{
	int i = 0;
	for(; i < 19; i++)
	{
		name[i] = nm[i];
	}
	name[i] = '\0';
	hobby = new char[strlen(ho) + 1];
	strcpy(hobby, ho);
	weight = wt;
}

Cow::Cow(const Cow & c)
{
	strcpy(name, c.name);
	name[strlen(name + 1)] = '\0';
	hobby = new char[strlen(c.hobby) + 1];
	strcpy(hobby, c.hobby);
	weight = c.weight;
}
name[strlen(name + 1)] = '\0';

That is certainly wrong.. and completely unnecessary, although I doubt it's the cause of your problems. The chief suspect at this point is your default constructor.
I can confirm that it didn't solve the problem. But yea, it also looks wrong to me when I think about it.
It only works when I don't use the default constructor...
1
2
3
4
5
6
Cow::Cow()
{
	name[0] = '\0';
	hobby = NULL;
	weight = 0.0;
}
You dereference `hobby' being NULL.
In order to avoid code to handle the special case where hobby is null, you may want to go ahead and allocate some memory in the default constructor.

1
2
3
4
Cow::Cow() : hobby(new char[1]), weight(0.0)
{
     name[0] = hobby[0] = '\0' ;
}
Topic archived. No new replies allowed.