JJWW
JJWW

Reputation: 13

While loop breaks before reading next line from input

I cannot for the life of me get this to work and I know it's going to be something stupid. This code is run on a button click. I want it to run and read multiple lines from a RichTextBox input and run the respective code, like a simple programming language that outputs shapes onto a canvas. So far all this does is run the first line on the input and breaks the loop.

Any help would be appreciated.

private void run_button(object sender, EventArgs e)
{
    int lineno = 0;
    int loopline = 0;
    int param = 0;
    string commandinit = commandbox.Text.Trim().ToLower();
    string[] lines = commandinit.Split('\n');

    while (lines[lineno] != null)
    {
        string[] command = lines[lineno].Split(' ', ',');

        if (command[lineno].Equals("moveto") == true)
        {
            if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int
            if (!Int32.TryParse(command[2], out positiony)) ;

            Canvas.xPos = positionx;
            Canvas.yPos = positiony;

            lineno++;
        }

        if (command[lineno].Equals("drawto") == true || command[lineno].Equals("draw") == true)
        {
            if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int
            if (!Int32.TryParse(command[2], out positiony)) ;

            Canvas.toX = positionx;
            Canvas.toY = positiony;
            MyCanvas.DrawLine(Canvas.toX, Canvas.toY);
            Refresh();//refresh display
            Console.WriteLine("COMMAND - LINE DRAWN");

            lineno++;
        }
        if (command[lineno].Equals("circle") == true)
        {
            if (!Int32.TryParse(command[1], out positionx)) ;

            Canvas.sizec = positionx;
            MyCanvas.DrawCircle(Canvas.sizec);
            Refresh();//refresh display
            Console.WriteLine("COMMAND - DRAW CIRCLE");

            lineno++;
        }

        else
        {
            Console.WriteLine("While loop broken");
            break;
        }
    }
}

Thanks for all your help guys. This is my updated code. It works up until the next line is blank, when it crashes on while (lines[lineno] != null) with System.IndexOutOfRangeException: 'Index was outside the bounds of the array.'

private void run_button(object sender, EventArgs e)
{
    int lineno = 0;
    int loopline = 0;
    int param = 0;
    string commandinit = commandbox.Text.Trim().ToLower();
    string[] lines = commandinit.Split('\n');

    while (lines[lineno] != null)
    {
        string[] command = lines[lineno].Split(' ', ',');

        if (command[0].Equals("moveto") == true)
        {

            if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int                    
            if (!Int32.TryParse(command[2], out positiony)) ;

            Canvas.xPos = positionx;
            Canvas.yPos = positiony;

            lineno++;
        }

        else if (command[0].Equals("drawto") == true || command[0].Equals("draw") == true)
        {
            if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int
            if (!Int32.TryParse(command[2], out positiony)) ;

            Canvas.toX = positionx;
            Canvas.toY = positiony;
            MyCanvas.DrawLine(Canvas.toX, Canvas.toY);
            Refresh();//refresh display
            Console.WriteLine("COMMAND - LINE DRAWN");

            lineno++;
        }
        else if (command[0].Equals("circle") == true)
        {
            if (!Int32.TryParse(command[1], out positionx)) ;

            Canvas.sizec = positionx;
            MyCanvas.DrawCircle(Canvas.sizec);
            Refresh();//refresh display
            Console.WriteLine("COMMAND - DRAW CIRCLE");

            lineno++;
        }

        else if (command[0].Equals("square") == true)
        {
            if (!Int32.TryParse(command[1], out positionshape)) ; //translate string to int

            Canvas.sizes = positionshape;
            MyCanvas.DrawSquare(Canvas.sizes);
            Refresh();//refresh display
            Console.WriteLine("COMMAND - SQUARE DRAWN");

            lineno++;
        }
               
        else
        {
            Console.WriteLine("While loop broken");
            break;
        }
    }

Upvotes: 0

Views: 147

Answers (5)

JJWW
JJWW

Reputation: 13

Ok it was working from a mixture of different answers which is in the edited code from the question. But changing it to a for loop using for (lineno = 0; lineno < lines.Length; lineno++)

updated working code is

private void run_button(object sender, EventArgs e)
        {
            int lineno = 0;
            int loopline = 0;
            int param = 0;
            string commandinit = commandbox.Text.Trim().ToLower();
            string[] lines = commandinit.Split('\n');

            for (lineno = 0; lineno < lines.Length; lineno++)
            {

                string[] command = lines[lineno].Split(' ', ',');

                if (command[0].Equals("moveto") == true)
                {

                    if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int                    
                    if (!Int32.TryParse(command[2], out positiony)) ;

                    Canvas.xPos = positionx;
                    Canvas.yPos = positiony;                                      

                }


                else if (command[0].Equals("drawto") == true || command[0].Equals("draw") == true)
                {
                    if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int
                    if (!Int32.TryParse(command[2], out positiony)) ;

                    Canvas.toX = positionx;
                    Canvas.toY = positiony;
                    MyCanvas.DrawLine(Canvas.toX, Canvas.toY);
                    Refresh();//refresh display
                    Console.WriteLine("COMMAND - LINE DRAWN");                   

                }
                else if (command[0].Equals("circle") == true)
                {
                    if (!Int32.TryParse(command[1], out positionx)) ;

                    Canvas.sizec = positionx;
                    MyCanvas.DrawCircle(Canvas.sizec);
                    Refresh();//refresh display
                    Console.WriteLine("COMMAND - DRAW CIRCLE");                    
                }

                else if (command[0].Equals("square") == true)
                {
                    if (!Int32.TryParse(command[1], out positionshape)) ; //translate string to int

                    Canvas.sizes = positionshape;
                    MyCanvas.DrawSquare(Canvas.sizes);
                    Refresh();//refresh display
                    Console.WriteLine("COMMAND - SQUARE DRAWN");                 

                }

                else if (command[0].Equals("rectangle") == true || command[0].Equals("rect") == true) //what happens if draw rectangle command is used
                {
                    if (!Int32.TryParse(command[1], out positionx)) ; //translate string to int
                    if (!Int32.TryParse(command[2], out positiony)) ;

                    Canvas.sizerx = positionx;
                    Canvas.sizery = positiony;
                    MyCanvas.DrawRect(Canvas.sizerx, Canvas.sizery);
                    Refresh();//refresh display
                    Console.WriteLine("COMMAND - DRAW RECTANGLE");                 

                }

                else if (command[0].Equals("colour") == true || command[0].Equals("col") == true || command[0].Equals("color") == true) //changes colour of the pen
                {
                    if (command[1].Equals("red") == true || command[1].Equals("r") == true)
                    {
                        Canvas.P1.Color = System.Drawing.Color.Red;                        
                    }
                    else if (command[1].Equals("blue") == true || command[1].Equals("blu") == true)
                    {
                        Canvas.P1.Color = System.Drawing.Color.Blue;                        
                    }
                    else if (command[1].Equals("black") == true || command[1].Equals("bla") == true)
                    {
                        Canvas.P1.Color = System.Drawing.Color.Black;                        
                    }
                    else if (command[1].Equals("green") == true || command[1].Equals("g") == true)
                    {
                        Canvas.P1.Color = System.Drawing.Color.Green;                        
                    }
                    else if (command[1].Equals("yellow") == true || command[1].Equals("yel") == true || command[1].Equals("y") == true)
                    {
                        Canvas.P1.Color = System.Drawing.Color.Yellow;                        
                    }


                }
                else if (command[0].Equals("loop") == true)
                {
                    loopline = lineno;                    
                }

                else if (command[0].Equals("loop") == true)
                {
                    loopline = lineno;                    
                }
                                
                else
                {
                    Console.WriteLine("While loop broken");
                    break;                    
                }

            }

                        
        }

Thanks for all your help

Upvotes: 0

r2d2
r2d2

Reputation: 637

Some suggestions:

  • while (lines[lineno] != null) is not working. You're looking for for(var i = 0; i < lines.Length; i++) or simply foreach(var line in lines).
  • if (command..) {..} if (command..) {..} else {..} will process all if conditions and and the else block for all commands except "circle". You're looking for if() {} else if() else {} or switch().
  • In the inner block: if(!Int32..); if(!Int32..); is not working because of semicolons at the end of the if and the missing braces around the following code block. You're looking for if(a && b){c}.

How about this code snipped to start with?


    string commandinit = "MoveTo 1 2\r\nDraw 1 2\r\n";
    string[] lines = commandinit.Split('\n');

    foreach (var line in lines)
    {
        string[] command = line.Trim().Split(' ', ',');

        switch (command[0].Trim())
        {
            case "MoveTo":
            {
                // ParseMoveToCommand(command);
                break;
            }
            case "Draw":
            {
                // ParseDrawCommand(command);
                break;
            }
            default:
            {
                Console.WriteLine("Unknown command");
                break;
            }
        }
    }

Upvotes: 1

Dan
Dan

Reputation: 36

I ran the code here and it does not execute "only the first line". The while loop do try to keep going. You are, however, double-incrementing the lineno variable For instance, for a "drawto" command: when you do a pass at a drawto if (which's true) you increment lineno. Then you move over to circle, which does nothing, and by the end you do a loopline = lineno (which is ok), but then you do lineno++ again, and that skips a line.

You will eventually move "past the bounds of the array". I'd change it from a while-loop to a for for-loop (since you already know how many lines you're executing).

for (lineno = 0; lineno < lines.Length; lineno++){ ... }

What happens to your code? Does it stop, gives you an exception, or will it simply close?

Edit: I'd also consider doing else if instead of several if statements. Edit2: Your else statement will run EVERY TIME the command is NOT "circle", and will break the loop (I removed the else from my code earlier for testing... sorry) . Consider the if/else suggestions from the @Hans Killian. -- I`m also new to this site so still learning how to answer..

Upvotes: 0

Hans Kilian
Hans Kilian

Reputation: 25144

Your indexing when you check for which command the line contains is wrong. You use lineno, but it should be 0 since you always want to look at the first word in the line. That's also why it works for the first line, because for the first line, lineno is 0.

So when you do

if (command[lineno].Equals("moveto") == true)

it should be

if (command[0].Equals("moveto") == true)

(and of course all the other places you use command[lineno]

Another issue is that your else at the bottom gets activated in all cases where the command isn't circle. To make sure you only terminate the loop when no valid command has been found, you should use else if and make the structure something like this

    if (command[0].Equals("moveto") == true)
    {
    }
    else if (command[0].Equals("drawto") == true || command[0].Equals("draw") == true)
    {
    }
    else if (command[0].Equals("circle") == true)
    {
    }
    else
    {
        Console.WriteLine("While loop broken");
        break;
    }

That will make it so you only get into the While loop broken section if none of the other ifs have matched.

You might want to use a switch statement instead of ifs and elses. It's a bit cleaner, imo.

Upvotes: 0

user7571491
user7571491

Reputation:

The statement if (command[lineno].Equals("circle") == true) should be else if (command[lineno].Equals("circle") == true) in addition what @Hans Killian suggested.

Upvotes: 0

Related Questions