problems making a constructor 1

[mikmaC]

Hi,
I am relatively new at c++. I am writing a code with
constructors and i'm with some problems in this part.
I'm using Microsoft Visual c++ 2008.

Next code compiles right, but there is problems with
execution.

I think it is because i'm not working right with
constructors and the strcpy statement does not its
right job.


#include <iostream>
using namespace std;

class caract{
char *temp;
public:
caract(void);
~caract(void);
void assign(char *str);
void show(void);
};

caract::caract(void)
{ temp = "hello ";}

caract::~caract(void)
{ cout << "destroyed\n";}

void caract::assign(char *str)
{ cout << temp;
cout << str << "\n";
strcpy(temp,str);
cout << temp << "\n";}

void caract::show(void)
{ cout << temp << "show \n";}

int main()
{ caract a, b, c;

a.assign("mike");
b.assign("however");
c.show();
return 0;}

If some one could help me, I appreciate.

 

[cpjust]

Since you didn't #include <cstring> that code shouldn't compile without at least some warnings. Make sure you set the compiler to Warning Level 4 (it defaults to 3 for some strange reason).

The problem is that you are assigning a constant string literal in the constructor and then in assign() you try to write into that constant which is read-only.
Allocate memory with new (and deallocate it with delete [] when you're done), or better yet, just use std::string (from the <string> header) and don't worry about new & delete.

 

[mikmaC]

could you show a example how I could make a Allocate memory with new and deallocate it with delete[].

thanks for your help.

 

[cpjust]

class String
{
public:
   String() : m_Str( NULL ) {};

   String( const char*  str )
   {
      m_Str = new char[ strlen( str ) + 1 ];
      strcpy( m_Str, str );
   }

   ~String()
   {
      delete [] m_Str;
   }

   const char* GetString()
   {
      return m_Str;
   }

   void SetString( const char*  str )
   {
      if ( m_Str )
      {
         delete [] m_Str;
         m_Str = NULL;
      }

      m_Str = new char[ strlen( str ) + 1 ];
      strcpy( m_Str, str );
   }

private:
   char*  m_Str;
};

 

[uolj]

Note that cpjust's example has incorrect copying. This is part of the reason you should use C++ strings.

If this is a learning exercise, you will want to learn about the rule of three and how to update that example to be safer.

 

[cpjust]

Incorrect copying? Where?

 

[uolj]

In the copy constructor and copy assignment operator.

Declare them as private or implement them correctly, otherwise that String class will probably cause a crash if it is ever copied.

I know it is just a quick example and that it is not intended to be perfect, but the incorrect copy behavior is a common problem when working with dynamic memory, which as you know is why the most recommended learning sources teach programmers how to not manage memory directly.

Since the OP's class will likely suffer from the same fate I figured it was worth it to point out.

 

[cpjust]

Oh yes, I didn't feel like writing an entire perfect class for this example. I thought you were referring to the parts where I was using strcpy() to copy the strings.
I guess it wouldn't have taken much longer to add 2 lines in the private section though.

 

[mikmaC]

About post above by uolj :
"In the copy constructor and copy assignment operator.
Declare them as private or..."


If I understood about last posts, we can't assignment an operator if that operator is private in a class?

If not, could you tell me how I can resolve this problem.


Although, seems to me uolj strongly recommends to work with C++ String.

 

[uolj]

If this is not a learning exercise specifically meant to learn about dynamic memory, you should definitely use the C++ string. You should be using the C++ string in all of your other projects of this level as well. I think cpjust agrees, as indicated by his first post.

As for the copying, yes, making them private will mean that you can't copy instances of your class. If you want to be able to copy, then you will have to implement them correctly. The default implementation just copies the pointer, which means that two different instances of the class will point to the same memory. Later, when both instances are destroyed that memory will be deleted twice, which is bad and can lead to a crash.

There are different solutions to implementing copying in these cases. One I like is to implement the copy constructor and a swap function, then implement the copy assignment operator in terms of those other two.

Another approach would be to write similar code in the copy constructor and copy assignment operator that allocates new memory in the object, then copies the string value over from the source. In the assignment operator you'll also have to check for self assignment and delete the original array.

 

[mikmaC]

Hi,

Thanks for your precious help. I did some progresses and I made the following code. In next example I declared variables type char with a pointer. It works fine.
Now, I'm not sure if I'm doing the right way because I'm using pointers for copying constructors. I need your
opinion about this.

#include <iostream>
using namespace std;

class caract{
char *temp;
public:
char *char1;
caract(void);
~caract(void);
void assign(char *str);
void show(void);
};

caract::caract(void)
{
temp = "hello ";
}

caract::~caract(void)
{
cout << "destroyed\n";
}

void caract::assign(char *str)
{
char *string = "hi ";
char1 = new char[strlen(str)+1];

strcpy(char1,string);
cout << "string ------>" << str << "\n";
strcat(char1,str);
cout << "cat String -->"<< char1 << "\n";
}

void caract::show(void)
{
cout << " ______________the end. \n";
}

int main()
{
caract a, b, c;

a.assign("mike");
b.assign("however");
c.show();
return 0;
}

 

[cpjust]

char1 = new char[strlen(str)+1];

You aren't allocating enough space for both str & "hi ".

Also, your destructor isn't deleting the memory you allocated to char1 (although since your default constructor doesn't use new, it would be impossible to tell when it's safe to delete it and when it's not).
Plus, since you call allocate() twice, allocate needs to delete char1 before allocating yet more memory that's going to get leaked...

Just use std::string and you don't need to worry about allocated/deallocating memory.