0

I have a very simple class with some char* members. Is strcat the best way to set their values?

class CImage {
public:
    CImage() {};

    char* Name;
};

void AddImage(const char* Name)
{
    CImage* img = new CImage();
    
    strcpy(img->Name, Name); // Is this correct or the best way?
}
bman
  • 5,016
  • 4
  • 36
  • 69

2 Answers2

11

Strongly advice you to drop the char * and use std::string.

With char * as member you need to bother about allocating(in constructor) and deallocating memory(in destructor) to the pointer yourself. Also, You need to manage the exceptions and do the manual management of the underlying data everytime you access it.
Also you would have to ensure following Rule of Three for your class.

In short there are too many places where you can shoot yourself in the foot(or head) easily, so safest and best way is to use a std::string as member and save yourself all the trouble.

Community
  • 1
  • 1
Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • Or he could use `vector`, if this array is used in C functions (Windows API), or the continuous memory allocation is a requirement. – Naszta Jun 15 '12 at 08:11
  • @Naszta: `std::string` is guaranteed to store its memory contiguously in C++11. Also, `std::string` has `c_str()` for C functions. – Jesse Good Jun 15 '12 at 08:23
  • @JesseGood: it is true in C++11. Unfortunately many product code are compiled older version of STL, what is not C++11 compliant. Several `std::string` implementation does not do it that way (optimized for short strings), so in multi-platform code it is better to use them as it described in the previous standard. In other hand: if his compiler supports C++11, he should use `std::string`. – Naszta Jun 15 '12 at 08:34
  • @JesseGood: One more thing: `c_str()` does not help you, when you get text from C function (e.g. [`RegQueryValueEx`](http://msdn.microsoft.com/en-us/library/windows/desktop/ms724911%28v=vs.85%29.aspx)). – Naszta Jun 15 '12 at 08:37
  • I was using string but AFAK string but I faced different problems. For example this one (http://stackoverflow.com/questions/11047089/access-violation-on-vector-push-back) I guess the problem might be Name property of sprite that cause Access Violation. I don't know why, I'm just skeptical. – bman Jun 15 '12 at 08:39
  • @Naszta: what are these several `std::string` implementations that don't store the string data contiguously? I ask because the C++0x committee did a straw poll before adding the new requirement to C++11 for contiguous string storage, and it did not find any. I agree that it's a bit of a last resort to rely on it in C++03, since it isn't guaranteed, but in fact it is true for all actively-maintained C++03 implementations despite not being guaranteed. – Steve Jessop Jun 15 '12 at 08:48
  • @SteveJessop: In [Scott Meyers: Effective STL](http://www.amazon.com/Effective-STL-Specific-Standard-Template/dp/0201749629) there is an item on this topic. Currently I am working on VS, so I don't know the current state of the other compilers. As I remember Borland compiler and intel compiler had this issue (couple of years ago). [Small String Optimization and Move Operations.](http://john-ahlgren.blogspot.hu/2012/03/small-string-optimization-and-move.html) – Naszta Jun 15 '12 at 08:56
  • @Naszta: I don't see what the small string optimization has to do with it. Either the string is small (and hence stored inside the string object) or it's large (and hence stored in an allocated buffer), but either way it's stored contiguously, and hence `&s[0]` points to an array of char containing the string data. It's only rope-like implementations of `std::string` that would cause problems, if there were any. – Steve Jessop Jun 15 '12 at 09:08
  • 1
    @Naszta: AFAIK, most(almost all) mainstream compilers already adhere to the contiguous memory allocation requirement.Using c-style strings in C++ only makes sense when you want to transfer the ownership to a c-style api or get it through one, Even in those cases One should wrap the raw data in a `std::string` if the member in question is a class member. – Alok Save Jun 15 '12 at 09:12
4

You must first allocate dynamically enough memory to store the string like this

img->Name = new char[strlen(Name) + 1];
strcpy(img->Name, Name);

And if you do so, you should then deallocate memory using delete[]

Also, you should consider incapsulating your string and its initialization in constructor.

The best way to store strings, however, is std::string

JustSomeGuy
  • 3,677
  • 1
  • 23
  • 31
  • I don't add minus, but this kind of allocation is not a good practice. You should use built in types (`vector`, `string` etc.) to handle memory automatically. – Naszta Jun 15 '12 at 08:13