Pointer Losing Value When Entering Function

Question

I'm trying to use my own String class to create a text based adventure game in the console. Although I'm having trouble with a pointer losing it's value. Here are cut down versions of the classes I think are causing the problem. ReadAndLog() sets the value of 'string' to the users input. When ToUpper() is called the value is still correct, however upon entering the length function the value at the location of 'string' is garbage data. Any insight into what is happening would be great. Thanks in advance.

String.h:

#ifndef STRING_H_
#define STRING_H_

class String
{
public:
    String();
    ~String();
    int Length();
    String & ToLower();
    String & ToUpper();
    void ReadAndLog();

private:
    char * string;
};

#endif

String.cpp:

#include "String.h"

String::String()
{
    string = nullptr;
}

String::~String()
{
    if (string != nullptr)
    {
        delete[] string;
        string = nullptr;
    }
}

int String::Length()
{
    if (string == nullptr)
        return 0;
    else
    {
        for (int i = 0; true; i++)
        {
            if (string[i] == '\0')
            {
                return i;
            }
        }
    }
}

String & String::ToLower()
{
    int length = Length();

    for (int i = 0; i < length; i++)
    {
        if (string[i] >= 'A' && string[i] <= 'Z')
            string[i] += 32;
    }

    return *this;
}

String & String::ToUpper()
{
    int length = Length();

    for (int i = 0; i < length; i++)
    {
        if (string[i] >= 'a' && string[i] <= 'z')
            string[i] -= 32;
    }

    return *this;
}

Edit - ReadAndLog Changed

void String::ReadAndLog()
{
    char charArray[256];
    std::cin.getline(charArray, 256);

    for (int i = 0; true; i++)
    {
        if (charArray[i] == '\0')
        {
            if (string != nullptr)
            {
                delete[] string;
                string = nullptr;
            }

            string = new char[i + 1];

            for (int j = 0; j < i; j++)
            {
                string[j] = charArray[j];
            }

            string[i] = '\0';
            break;
        }
    }

    File::LogEntry((String)"User", string);
}

The code that is calling is in Game.cpp which calls:

String input;
input.ReadAndLog();
input.ToUpper();

_`delete[] string;`_ Where did you actually ever allocated memory for your `string`? I can't spot it from your sample. — πάντα ῥεῖ, Apr 04 '15 at 18:35
My apologies, I had cut that part out. I have another constructor where I dynamically allocate the memory from a char[] parameter. — user1929613, Apr 04 '15 at 18:41
Anyway, the marked duplicate explains what's wrong with your `ReadAndLog()` code. Rather use `std::vector` or any other appropriate c++ standard container. — πάντα ῥεῖ, Apr 04 '15 at 18:44
I see completely what I did now it was pointed out exactly where it was happening, I've edited the code and it seems to work, Am I forgetting anything else? any stray pointers or anything? — user1929613, Apr 04 '15 at 18:50
_"Am I forgetting anything else? "_ If you use `new`, take care to have a corresponding `delete` somewhere else. Good luck with getting that right. I'd still recommend `std::vector` that does it correctly already. See here for more advanced info please: http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — πάντα ῥεῖ, Apr 04 '15 at 18:53
This is for a school assignment where we are to re-create std::string and create a text based game from it otherwise I would be :) thanks for the help. — user1929613, Apr 04 '15 at 18:55
If you're forced to implement it yourself, you should thoroughly read through that "Rule of 3" link I gave you. — πάντα ῥεῖ, Apr 04 '15 at 18:57

score 0 · Answer 1 · answered Apr 04 '15 at 18:34

0

char charArray[256] is local to ReadAndLog. It gets disposed of when the function exits. You need to allocate a new blob of memory (and dispose of it in your destructor) or even better, use a std::string.

answered Apr 04 '15 at 18:34

Robinson

9,666
16
71
115

score 0 · Answer 2 · answered Apr 04 '15 at 18:35

0

This

char charArray[256];
//...
string = charArray;

can't work. You are essentially assigning the address of a local variable to a member pointer of your string class. Later you are trying to access that local variable via the pointer, but at that point in time, the variable has gone out of scope and you are accessing an invalid memory address.

You probably should familiarize yourself with memory management in c++.

answered Apr 04 '15 at 18:35

MikeMB

20,029
9
57
102

So instead I should count the length of charArray, then say string = new char[length] and then loop through each value of charArray? Is this correct? – user1929613 Apr 04 '15 at 18:39
@user1929613: What you should do is use a `std::string` internally. – MikeMB Apr 04 '15 at 19:30

Pointer Losing Value When Entering Function

2 Answers2