Return to Answer

• 1

• 50.9k
• 5
• 83
• 177

The code in question has some flaws which I will address here:

• var index = new List<int>(); isn't really used and should be removed
• var value = new List<String>(); isn't really used and should be removed
• The method parameter List<String> list isn't used as a List<T> but rather like an array hence it should be an array.

Fixing these points leads to

private static int CountIfs(string[] linesOfCode)
{
 var count = 0;
 for (int i = 0; i < linesOfCode.Length; i++)
 {
 var line = linesOfCode[i];
 if (line.Contains("if"))
 {
 if (line.Contains("then") || line[i + 1].Contains("then"))
 {
 count++;
 }
 }
 }
 return count;
}

Now the code looks much cleaner and smaller.

• An if will always be the first statement in a line of code after removing any leading whitespace and will have a trailing whitespace. Using Contains() can lead to many false positives. Assume a line of code contains the following text:
  "I received the tif-file of this invoice from my partner in athen."

• A then will always have a leading whitespace and either have a trailing whitespace or will be the first part of a line of code after removing leading whitespace.

• Like mentioned in [@Paparazzi's answer][1] you could run into an IndexOutOfRangeException by accessing list[i + 1].

Let us introduce a new method which just handles the check for the if statement.

private static bool HasIfStatement(string line)
{
 return line.Trim().StartsWith("if ");
}

No we need a method to check for a then.

private static string[] spaceArray = new string[] { " " };
private static bool HasThenStatement(string line)
{
 return line.Split(spaceArray , StringSplitOptions.RemoveEmptyEntries).Any(w => w == "then");
}

Well this method is some kind of quick & dirty because it will create a lot of strings. I will leave it to you to make it better.

By adding a flag for the case that the line had an if and doesn't had an then we can't get an IndexOutOfRangeException anymore. As a sideeffect we could now use a foreach loop and have an IEnumerable<string> as the methods parameter type.

Putting all together could look like so

private static int CountIfs(IEnumerable<string> linesOfCode)
{
 var count = 0;
 var lastLineHadIf = false;
 foreach (var line in linesOfCode)
 {
 if (lastLineHadIf)
 {
 lastLineHadIf = false;
 if (HasThenStatement(line))
 {
 count++;
 }
 }
 else if (HasIfStatement(line))
 {
 if (HasThenStatement(line))
 {
 count++;
 }
 else
 {
 lastLineHadIf = true;
 }
 }
 }
 return count;
}
private static bool HasIfStatement(string line)
{
 return line.Trim().StartsWith("if ");
}
private static string[] spaceArray = new string[] { " " };
private static bool HasThenStatement(string line)
{
 return line.Split(spaceArray , StringSplitOptions.RemoveEmptyEntries).Any(w => w == "then");
} 

I don't know about case-sensitivity of if and then so you may adjust the code to take this into account.

This code doesn't take comments into account, so you should adjust the code for comments as well. [1]: https://codereview.stackexchange.com/a/187624/29371