11

In my application, I create a char* like this:

class sample
{
    public:
        char *thread;
};

sample::sample()
{
    thread = new char[10];
}

sample::~sample()
{
    delete []thread;
}

Am I doing the right thing in the code?

karthik
  • 17,453
  • 70
  • 78
  • 122
  • why `[]thread` and not `delete thread;` only? – mauris Mar 30 '11 at 03:24
  • 1
    Do you mean `thread = new char[10]`? – sth Mar 30 '11 at 03:25
  • @thephpdeveloper Because he wants to delete the data that is "pointed" to by thread. The * before thread means its a pointer. The pointer thread will be "deleted" when it falls out of scope. – Pete Mar 30 '11 at 03:26

3 Answers3

27

If you have [] after your new, you need [] after your delete. Your code looks correct.

Fred Larson
  • 60,987
  • 18
  • 112
  • 174
14

List of points to be noted:

1) You need to allocate room for n characters, where n is the number of characters in the string, plus the room for the trailing null byte.

2) You then changed the thread to point to a different string. So you have to use delete[] function for the variable you are created using new[].

But why are you fooling around with new and delete for character data? Why not just use std::string, instead of 'C' functions? It's amazing why so many don't do the easiest thing:

#include <cstdio>
#include <string>

int countWords(const char *p);

int main(int argc, char *argv[])
{
    std::string pString = "The Quick Brown Fox!";

    int numWords1 = countWords(pString.c_str());
    printf("\n\n%d words are in the string %s", numWords1, pString.c_str());

    int numWords2 = countWords(argv[1]);
    printf("\n%d words are in the string %s", numWords2, argv[1]);
}

No need for new[], delete[], strcpy(), etc.

Use strlen(). Better yet, don't use char* and use std::string for string data.

fredoverflow
  • 256,549
  • 94
  • 388
  • 662
karthik
  • 17,453
  • 70
  • 78
  • 122
  • 6
    Don't use `strlen()` with `std::string`. Use the `.size()` member function - this will work correctly even if the string has embedded null characters. As an added bonus, it's O(1)! – bdonlan Mar 30 '11 at 03:36
  • but isn't std::string generally not thread safe "http://stackoverflow.com/questions/1661154/c-stdstring-in-a-multi-threaded-program" so it depends on his requirement to use char* or std::string. – user258367 Mar 30 '11 at 05:11
  • 1
    @user258367: No. The very worst case is that you'd have to suppress COW in multi-threaded code by copying a `std::string` via `const char* string::c_str()` and `string::string(const char*, size_t)` to eliminate the COW. Still a whole lot safer than 100% manual string handling. – MSalters Mar 30 '11 at 16:21
2

It's "right"*, but it's very wrong.

You should not use new[], but instead use std::vector<char> or std::string. Even if you weren't doing that, you need to respect the rule of three, or your class is broken.

*Assuming you meant new char[10]. Also, more orthodox is delete[] thread.

Community
  • 1
  • 1
GManNickG
  • 494,350
  • 52
  • 494
  • 543