Loop test itterates to many times.

[Fuzz13]

I am writing a program which reads a file into an array creating a member for every line of text. I am trying to create a test that will examine each member for a given list of char's and IF any of the listed char's are found it needs to spit out an error. If no errors are found it needs to go to the next set of instructions. It seems like a simple If Else statement but in order to iterate for every line I'm using a for loop as such:

if (cleancheckbox.Checked == false)
                {

                    // Return an error for file with illegal characters
                    for (int i = 0; i < lines.Length; i++)
                    {

                        if (lines[i].IndexOfAny(illegal) != -1)
                        {
                            newobject.illegalError();
                            break;
                        }


                    // If file has no illegal characters this creates the files.

                        else
                        {
                            // stores the string entered for the new folder name
                            newobject.ParentFolder = newfldrbx.Text;

                            // Specify a folder to house everything new inside
                            //This is not the folder that holds the created files/folders
                            // based off of csv/txt values
                            string outputdir = folderBrowserDialog1.SelectedPath;

                            //Create a new subfolder under the current active folder
                            // this new folder will hold all files/folders created
                            // per project.
                            string newPath = System.IO.Path.Combine(outputdir, newobject.ParentFolder);

                            // Create the subfolder
                            System.IO.Directory.CreateDirectory(newPath);

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

                                //takes the input from the new folder text box
                                // and assigns it to the filename variable
                                string fileName = lines[j] + ".txt";

                                // creates new file based off of file name input annd
                                // parent directory
                                string newFile = System.IO.Path.Combine(newPath, fileName);
                                System.IO.File.Create(@newFile);
                      
                            }

The problem, if I am understanding correctly, is that having my if else inside the FOR loop is causing it to read every line for the given char's and if it fails the IF statement, it goes to the ELSE and loops repeatedly until the FOR condition is met.

I think a Break; would work to cause it to only run thru the ELSE statement once but I can't figure it out exactly.

If anyone has a solution to this or a recommendation to an alternative method (I toyed with a while loop but got even more lost) I would be very appreciative.

Thanks in advance.

[GaryTexmo]

I don't understand why you have two loops here. You've got an outer for loop that reads through every item in lines and performs a test to see if the current item contains illegal characters or not. That makes sense to me, but then you go on to do another loop through all the lines and create the files.

With this logic, lets say you had 4 lines with the first one being illegal. On the second pass through the outer loop, you'll enter into the else condition which will then still attempt to create the first file, along with the second, third, and fourth. The 3rd iteration will attempt to create all four again, as well as on the last iteration.

Can you not remove that loop in the else block and simply perform your operation on the current item (lines[i])?

Perhaps I'm missing something or not reading your code correctly. My apologies if that's the case.

[Fuzz13]

Thank you for your reply. The reason for the 2 loops is that (and mind you I am very new to this so it is the only way I could figure it out):
I use the for loop to read each line. From what I can tell I need something like a for loop where "i" increases each time so it will continue to read line by line.
But the "for" loop is not a test, it is only causing it to go thru each line. The if-else test is my test. I understand what you're saying, i read the lines then no matter what it attempts to create the files as well.

Ultimately I want it to read each line to test it for illegal characters, but then IF there are illegal characters it needs to return my error (newobject.Ille galError) and if there are no illegal characters it needs to go to my ELSE and create the files.

I hope that helps clear it up a little. Thanks.

[GaryTexmo]

Right, so in your else condition I think you are fine without the second loop. Can you just work on the line from the top for loop?

As pseudo code...

for each line in files
{
  if (line contains illegal chars)
  {
    output error
  }
  else
  {
    create file with name of line
  }
}

[Fuzz13]

I see what you're sugesting which definately is one solution however I idealy want the initial test to read every line and if at any point any line fails the test I don't want it to do my else statement. By following your pseudocode what happens is it goes line by line spitting out errors for each failure and creating files for each line that passes. How do I set it up to test for failures first and if there are zero failures then create based off the lines?

[GaryTexmo]

You want it to stop when it encounters an error? If that's the case, use "break" to stop a loop.

for (int i = 0; i < 10; i++)
{
  if (i == 5) break;
}

This is a for loop that goes from 0 to 9, but when i is 5, it stops the loop prematurely.

If you want to test every line first, then if they are all ok, do your operations on them, you'll need two separate loops. In your original code you have nested loops, which generates length * length operations. You want length * 2 operations.

bool linesOK = false;
for each line in files
{
  if (line contains illegal chars)
  {
    linesOK = false;
    break;
  }
}

if (linesOK == true)
{
  for each line in files
  {
    do whatever
  }
}

[Fuzz13]

BAM! That was perfect. Thank you so much. I knew there was a moderately simple solution I was missing which was what you gave me (of course by no means am I trying to undercut the genious of the solution you gave :) )

I appreciate you working thru this with me greatly!

[GaryTexmo]

You're quite welcome :)

In the future, when it comes to loops, it really helps to sit down and trace through them. Figure out what they're doing and try to compare that with what you want them to do.

Sometimes tracing through it with a pen and paper just helps get your mind around things.

Good luck with the rest of your project!