Allocate Memory

[msg555]

I'm writing a String class, but I find I'm having trouble allocating memory. It contains two member variables, m_Length (int; the length of the string, not including the null at the end) and m_Char (* char to the contained string)

Upon debugging, I find that the same memory location is set to m_Char everytime for every instance of the class. Obviously this is a huge problem because every string I create points to the same string.

This is the function I created to allocate memory

void zString::Alloc(const unsigned int & Bytes)
{
if (m_Length) delete [] m_Char;
if (Bytes)
{
m_Char = new char[Bytes+1];
for(int i=0; i<=Bytes + 1; i++)
m_Char[ i ] = '\0';
}
m_Length = Bytes;
}

By the way, I like the look of this forum

[Paul McKenzie]

Is this for school? If not, there is already a string class in C++, std::string.

Regards,

Paul McKenzie

[Paul McKenzie]

Also, please post your entire class and the program you are running to test it. Post only a small main() program that duplicates the problem.

Did you write a copy constructor and assignment operator for your string class? This is probably where your problem is.

Also, if you don't have to do this, again, use std::string. It is already written, so there is no need to reinvent the wheel.

Regards,

Paul McKenzie

[msg555]

Allright.

Well, I might use the std::string class. Where can I learn how to use it?

Even if I decide to do that, I'd like to know what I did wrong. (I did write a copy constructor)

also, I'm using Dev-cpp if it matters

[Paul McKenzie]

Allright.

Well, I might use the std::string class. Where can I learn how to use it?

Practically every C++ book discusses usage of std::string. If the book is good, it should be in chapter 1 or 2.

I will look at your code later.

Regards,

Paul McKenzie

[Paul McKenzie]

One question:

Why aren't you using, at the very least, the basic <string.h> functions in your class or even functions such as memcpy() when copying one buffer to another?

For example:

zString::zString(const char * const rhs)
{
    for(m_Length=0; *(rhs+m_Length)!='\0'; m_Length++) {} //Find first Null
    Alloc(m_Length);
    for (int i=0; i<m_Length; i++)
        (*this)[i] = *(rhs+i);
}

There is no need to code loops to find the length of a null-terminated string, or loops to copy data.

zString::zString(const char * const rhs)
{
    m_Length = strlen(rhs);
    Alloc(m_Length);
    memcpy(m_Char, rhs, m_Length);
}

Regards,

Paul McKenzie

[msg555]

I can only answer your question with a simple, "because I feel like it"
Is there any real advantage to coding it that way?

Also, I read "Sams teach yourself c++" book. It nevered discusses std::string, although it go over some of the function in string.h like memcpy

[Paul McKenzie]

I can only answer your question with a simple, "because I feel like it"

That is not the way to approach programming in C++. We will give you answers based on the experience that we have here, which is not based on "feel" but on what works and is used by professionals. When another C++ programmer looks at your code, they will also wonder why you didn't use the library routines. It isn't just myself that will say this.

Is there any real advantage to coding it that way?

If you are talking about using the standard library, yes. It is already written for you, it is standard, it is as efficient as possible, it has no bugs, and other C++ programmers can understand the code much more quickly, since the functions used are standard. If I took a quick glance, I have no idea if those loops you wrote are correct. However, if I saw a simple call to strlen() or memcpy(), then this is fully understandable to myself and other programmers what those functions do.

Also, I read "Sams teach yourself c++" book. It nevered discusses std::string,

Throw the book away. It is either outdated (pre 1998), or it is a piece of junk. It isn't anything other than those two options.

Regards,

Paul McKenzie

[Paul McKenzie]

There are many things wrong with your class. Here are three of them:

1) You should initialize the m_Char to NULL on construction of the zString object.

2) Your assignment operator does not return a value

3) Why does your destructor call Alloc()? Why not just call delete []?

#include <string.h>

zString::~zString()
{
    delete [] m_Char;
}


zString::zString(const char * const rhs) 
   : m_Char(0)  // Do this for each constructor
{
    m_Length = strlen(rhs);
    Alloc(m_Length);
    memcpy(m_Char, rhs, m_Length);
}

bool zString::operator==(const zString & rhs) const
{
    return (strcmp(m_Char, rhs.m_Char) == 0);
}

zString & zString::operator= (const zString & rhs)
{
   Alloc(rhs.Length());
   strcpy( m_Char, rhs.m_Char);
   return *this;
}

void zString::Alloc(const unsigned int & Bytes)
{
    delete [] m_Char;
    m_Char = new char[Bytes+1];
    memset(m_Char, 0, Bytes+1);
    m_Length = Bytes;
}

Just these changes alone is all that I needed to get this to work. But I would never use this, since again, std::string is there so that you don't need any of this code.

Regards,

Paul McKenzie

[msg555]

Thanks for you help Paul.

You should initialize the m_Char to NULL on construction of the zString object.

I'll be sure to add that.

Your assignment operator does not return a value

lol, that is going to be a problem.

[msg555]

Allright, so the problem is caused by not setting m_Char to NULL. I don't get why that caused a problem considering every constructor calls Alloc and Alloc set's m_Char to a new block of memory.

[Paul McKenzie]

Allright, so the problem is caused by not setting m_Char to NULL. I don't get why that caused a problem considering every constructor calls Alloc and Alloc set's m_Char to a new block of memory.

Please look carefully at your Alloc code, and see what I changed it to. The very first line in Alloc is in error:

 if (m_Length) 
     delete [] m_Char;

You set m_Length to a value, and then you are attempting to call delete on an invalid pointer. m_Char has a garbage value when the constructor was called, and calling delete [] on this value leads to unpredictable results. That is why setting it to NULL in the member-initialization list ensures that calling delete will not have any effect (calling delete on a NULL pointer is valid).

This is why std::string was developed, so you don't need to write code like this and cause bugs to occur.

As to books:

http://www.accu.org/bookreviews/public/index.htm

"C++ Primer Plus" - Lippman
"Accelerated C++" - Koenig & Moo
"The C++ Standard Library" - Josuttis
"The C++ Programming Language 3rd edition" - Stroustrup

Regards,

Paul McKenzie