Stuck in infinite loop while trying to remove a char

Question

I am working on a project that translates english to pig latin to better learn strings. The rules I'm struggling with are:

If a word starts with a vowel, just add way to the end of the word.
If a word starts with a consonant, move the consonants before the first vowel to the end of the word and add ay.

I can get the vowels to work, but I'm get stuck in an infinite loop if I hit a constant. I am fairly new to programming and C#. Any help figuring this out would be appreciated and hopefully help me understand the res.

I have tried creating a char using text and an int using text to check each letter. Not really sure what to do.

This is the translate button code:

private void transBtn_Click(object sender, EventArgs e)
{
    english = engTxtBx.Text;
    english = english.Trim();
    string[] columns = english.Split(' ');
    for (int i = 0; i < columns.Length; i++)
    {
        string text = columns[i];
        char first = text[0];
        for (int c = 0; c < text.Length; c++)
        {
            char character = text[c];
            //int consonant = text.IndexOf(character);
            if (isVowel(first))
            {
                text = text + "way ";
                pigLatin = text;
                plTxtBx.Text += text;
            }
            else if (!isVowel(character))
            {
                if (isVowel(text[c + 1]))
                {
                    text.Remove(text.IndexOf(character), 1);
                    text += character + "ay";
                    pigLatin = text;
                    plTxtBx.Text += text;
                }
                break;
            }
        }
    }

public static bool isVowel(char c)
{
    return new[] {'a','e','i','o','u'}.Contains(char.ToLower(c));
}

If I type the phrase "Can I have an apple" the see should be removed from the first char and moved to the end and "ay" should be added on after. Right now when I debug the c is not removed, but added to the end and ay is added. Then it gets stuck on "I".

jhilgeman · Accepted Answer

So Robert provided a fix, but since you said you were new to programming, I'll try to explain the problem, and I'll try to do it with a simplified example of what you're doing:

string text = "Hello!";
for(int i = 0; i < text.Length; i++)
{
     text += "!";
}

Your loop condition is based on the text.Length, but within the loop, you're appending to that text, so every time the loop gets ready to start again, it checks text.Length and says, "Well, I'm -STILL- not at the end of text.Length, so I guess I'll keep looping.

If you want to loop only based on the initial number of characters, you should either:

A. Store the text.Length into a separate variable BEFORE the loop, and then base your loop condition off of it, like this:

string text = "Hello!";
int initialLength = text.Length;
for(int i = 0; i < initialLength; i++)
{
     text += "!";
}

B. Or even better, instead of appending more content onto your text variable, use a separate string variable as your "modified" copy:

string text = "Hello!";
string text2 = text;
for(int i = 0; i < text.Length; i++)
{
     text2 += "!";
}

C. Or best yet, take approach B but use the StringBuilder class. In C#, when you modify a string, the system is creating a brand-new COPY of your string in memory along with the extra characters, so this:

text = "Hello";
text += "!";
text += "!";
text += "!";

...is actually creating four different string variables in memory:

Hello Hello! Hello!! Hello!!!

It's a temporary thing but it's inefficient and wasteful. The StringBuilder class is meant to allow you to efficiently build a string, piece by piece:

StringBuilder sb = new StringBuilder("Hello");
sb.Append("!");
sb.Append("!");
sb.Append("!");
Console.WriteLine(sb.ToString()); // "Hello!!!"

So if you looked at example B, you might change it to:

string text = "Hello!";
StringBuilder sbText = new StringBuilder(text);
for(int i = 0; i < text.Length; i++)
{
     sbText.Append("!");
}

My final recommendation is to also get the string / content the way you want it to be and THEN assign it to your control. So don't call this 10 times:

 plTxtBx.Text += text;

Instead, get your text set up the right way, and then at the end, just do a simple assignment:

 plTxtBx.Text = final_text;

The reason for this is that controls have various events and little cogs and gears that run when you modify a property like Text. If you do the += trick to append text to the Textbox control, then you're forcing the control to run through its whole routine every time you update the text.

These are small inefficiencies but they add up over time, so it's good practice to do it right upfront.

So if I took Robert's proposed code (I haven't tested it personally) and changed it a bit, I would do:

private void transBtn_Click(object sender, EventArgs e)
{
    // Start with whatever is in plTxtBx
    StringBuilder sb = new StringBuilder(plTxtBx.Text); 

    // Break into words
    string[] columns = engTxtBx.Text.Trim().Split(' ');
    for (int i = 0; i < columns.Length; i++)
    {
        if (isVowel(columns[i][0]))
        {
            // Start with vowel.
            sb.Append(columns[i]);
            sb.Append("way");
        }
        else
        {
            // Start with consonant. Get index of first vowel.
            int index = columns[i].IndexOfAny(vowels);
            if (index == -1)
            {
                // No vowel in columns[i].
                // You have to decide what to do.
            }
            else if (index == 1)
            {
                // First vowel is the second letter.
                sb.Append(columns[i].Substring(1));
                sb.Append(columns[i][0]);
                sb.Append("way");
            }
            else
            {
                // First vowel is after the second letter.
                sb.Append(columns[i].Substring(index));
                sb.Append(columns[i].Substring(index - 1, 1));
                sb.Append("way");
            }

        }
    }

    // Update plTxtBx's Text once with the final results
    plTxtBx.Text = sb.ToString();

}

Stuck in infinite loop while trying to remove a char

Answers (2)

Related Questions