[Solved] Pressing Backspace in my typing function sometimes crashes my program

[DraGun]

so i made a thread about a month and a half ago asking for help on how to make a typing function. Well, a little while ago it started just randomly breaking sometimes. Here's the function:

bool typeText(sf::Clock & clock, sf::String & stringSF, sf::Event & event, std::string & string)
{

        bool finished = false;

        if (event.type == sf::Event::TextEntered && clock.getElapsedTime() >= sf::milliseconds(120)
                && event.text.unicode != '\b' && event.text.unicode != '\r')
        {
                clock.restart();
                stringSF.insert(stringSF.getSize(), event.text.unicode);
        }


        if (event.type == sf::Event::TextEntered && event.text.unicode == '\b' && clock.getElapsedTime() >= sf::milliseconds(100)
                && stringSF.getSize() != 0)
        {
                clock.restart();
                string.erase(stringSF.getSize() - 1, 1);
                stringSF = string;
        }


        if (event.type == sf::Event::TextEntered && event.text.unicode == '\r')
        {
                clock.restart();
                string = stringSF;
                finished = true;
        }

        return finished;

}

[Arcade]

What do you mean by breaking? Is it causing your program to crash?

It is a bit hard to tell what is going on because it isn't obvious what the arguments of your function mean (it's also  a bit confusing to have a string variable called "string"). If I were to guess, though, I would say your problem is this line:

string.erase(stringSF.getSize() - 1, 1);
 

Is string guaranteed to be at least the same size of stringSF or larger? If not, this line is problematic.

[Jesper Juhl]

I don't know exactely what you are trying to do, but a few comments on the code you posted:

1. Since you test for "event.type == sf::Event::TextEntered" in every if statement you could just do an early test and return at the top of the function - like

if (event.type != sf::Event::TextEntered)
return finished;

2. Since it doesn't look like you expect to enter more than one of the branches in your function, you should probably use 'else if (...)' rather than multiple 'if (...)' statements.

[DraGun]

@Arcade: Yes it is causing it to crash, as the title says. But it's only sometimes, it's very random. Well, it's called string so you know what to input.... it's just a function . Yes they are to remain the same size. The point of this function is dynamic typing with text that is on screen, part of the UI I'm making.

@Jesper: I could, but i normally have bad experiences with 'else's, they'll just not work for me sometimes, so I just usually play it safe and go with another 'if.' It seems the 'if' statement you're wanting to add just adds another comparison.

[Arcade]

@Arcade: Yes it is causing it to crash, as the title says.

Doh. By the time I read the body of your message I forgot that the title mentioned it crashing 

I don't see anything in the function itself to cause a crash, but I do see how it could crash depending on the input arguments of the function. Here is a simple trick you can try if you haven't already: how about at the start of your function put in some prints to see what "string" and "stringSF" really are. You can also put in some prints within your if statements. For example:

std::cout << "String: " << string << std::endl;

When your program crashes you can see the last things that got printed which may give a clue as to what happened. Alternatively, if you know how to use a debugger, you can just use that to see which line is causing the crash.

[Hapax]

I would guess that the 'crash' is an out-of-range exception being thrown?
It looks like you're not keeping the std string and the sf string equal. You're adding characters (stringSF.insert()) but not adding them to the std string. Then, you erase from the std string based on the length of the sf string which is not the same because the characters haven't been added to the std string. Therefore you're trying to erase from the std string when there are no characters to erase.

[DraGun]

Well, this is where the debugger is finding the problem:



I think what Hero said about them not being equal is the problem, I'm gonna try that.

EDIT: I see that I just failboated on posting a screenshot of the debugger. Halp?

[Hapax]

After

stringSF.insert(stringSF.getSize(), event.text.unicode);

try adding

string = stringSF;

[DraGun]

@Hapax: Sorry for calling you Hero earlier, I mis-remembered your name XD confused it with your forum rank ;P
Anyways, that combined with also erasing from the string seems to have fixed it. Thanks guys! This has been a recurring problem that I've been pushing off, and working on for hours.

[Hapax]

After you erase from string, you're setting stringSF to equal it so there's no need to erase from it 

Since you're just removing the last character from the string, you may want to consider just using string.pop_back(). If not, at least use the size of string rather than stringSF (this is why it was crashing when they weren't equal).

[DraGun]

Yeah I fixed the inequality in the function. Huh, didn't know that was a function (just finished my freshman year studying for computer science )

[Jesper Juhl]

@Jesper: I could, but i normally have bad experiences with 'else's, they'll just not work for me sometimes, so I just usually play it safe and go with another 'if.'

Not trying to be rude, but that sounds like you should spend some more time with a good C++ book.
The else statement does exactely what its name implies; if the main (if) branch is not taken the else branch is. There is no magic or ambiguity there.

 

It seems the 'if' statement you're wanting to add just adds another comparison.

Let's compare.
Here's how you wrote the function:

bool typeText(sf::Clock & clock, sf::String & stringSF, sf::Event & event, std::string & string)
{

    bool finished = false;

    if (event.type == sf::Event::TextEntered && clock.getElapsedTime() >= sf::milliseconds(120)
        && event.text.unicode != '\b' && event.text.unicode != '\r')
    {
        clock.restart();
        stringSF.insert(stringSF.getSize(), event.text.unicode);
    }


    if (event.type == sf::Event::TextEntered && event.text.unicode == '\b' && clock.getElapsedTime() >= sf::milliseconds(100)
        && stringSF.getSize() != 0)
    {
        clock.restart();
        string.erase(stringSF.getSize() - 1, 1);
        stringSF = string;
    }


    if (event.type == sf::Event::TextEntered && event.text.unicode == '\r')
    {
        clock.restart();
        string = stringSF;
        finished = true;
    }

    return finished;

}
 

In the case where the function is called with an event where .type is sf::Event::TextEntered I count 3 comparisons.
In the case where the function is called with an event where .type is not sf::Event::TextEntered I count 3 comparisons.

Here's how I'd write the function (at least regarding what we are discussing here):

bool typeText(sf::Clock& clock, sf::String& stringSF, sf::Event& event, std::string& string)
{
    if (event.type != sf::Event::TextEntered)
        return false;

    bool finished = false;
    if (clock.getElapsedTime() >= sf::milliseconds(120) && event.text.unicode != '\b' && event.text.unicode != '\r') {
        clock.restart();
        stringSF.insert(stringSF.getSize(), event.text.unicode);
    } else if (event.text.unicode == '\b' && clock.getElapsedTime() >= sf::milliseconds(100) && stringSF.getSize() != 0) {
        clock.restart();
        string.erase(stringSF.getSize() - 1, 1);
        stringSF = string;
    } else if (event.text.unicode == '\r') {
        clock.restart();
        string = stringSF;
        finished = true;
    }

    return finished;
}
 

In the case where the function is called with an event where .type is sf::Event::TextEntered I count 1 comparison.
In the case where the function is called with an event where .type is not sf::Event::TextEntered I count 1 comparison.
Also consider the fact of the many fewer comparisons of all the stuff that is not event.type due to the use of else if.