C# - Can't get the correct result using EndofLoop

Question

I'm using C# to compare data in different sheets in excel, and need to write the pass/fail result into the excel as well. But for this part code, I only get the result of 'fail' in the excel, even the result should be 'pass'. Anyone could have a look please?

for (i = 1; i <= baseline.GetLength(0); i++)
    for (j = 1; j <= baseline.GetLength(1); j++)
        {
          if (baseline[i, j]?.ToString() == result_PriceUpdate[i, j]?.ToString()) goto EndofLoop1; 
          if (baseline[i, j]?.ToString() != result_PriceUpdate[i, j]?.ToString()) goto EndofLoop2;
        }

   EndofLoop1:
   result_file.WriteResultToExcel("Result Summary", 2, 2, "pass");

   EndofLoop2:
   result_file.WriteResultToExcel("Result Summary", 2, 2, "fail");

You are always exiting the loop after the first comparison. You don't check all the values at all. — NineBerry, Jan 31 '19 at 01:22
Looks like the line you use to write fail to your spreadsheet is always working last and overriding anything on that cell. — arunes, Jan 31 '19 at 01:23
yep, figured it out already..so how can i get the correct result using this way please? — HCYH, Jan 31 '19 at 01:24
What is the correct result? Do you intend to only check `baseline[1,1]` or what? — Blorgbeard, Jan 31 '19 at 01:25
You should get rid of the `goto` statements to start with. Those are prematurely exiting your `for` loops. You can replaces them with the actual code from the label or put the code in a method and call it from the `if`. It's also good practice to include the braces around the outer `for` loop as well. Also, just want to be sure you're aware that collections are `0` based, not `1` based (you are starting at the second element when you do `i = 1`). — Rufus L, Jan 31 '19 at 01:26
It's not clear what you want, then. Since `baseline[1, 1] != result_PriceUpdate[1, 1]`, you will always get a fail result. — Rufus L, Jan 31 '19 at 01:31
if baseline [1,1] != result_priceupdate[1,1], it should be fail. but expected result is they are the same value there, which should be pass — HCYH, Jan 31 '19 at 01:32
The easiest way to see what's happening is to set a breakpoint on the first `if` statement, and then examing the values of `baseline[1, 1]` and `result_PriceUpdate[1, 1]`. In the Immediate window you can execute something like ``baseline[1, 1] != result_PriceUpdate[1, 1]`` and see if it returns `true` or `false`. There may be an extra space at the end of one of them, or the casing may be different. You can also change your code to `if (baseline[i, j]?.ToString().Trim().Equals(result_PriceUpdate[i, j]?.ToString().Trim(), StringComparison.OrdinalIgnoreCase) ?? false);` — Rufus L, Jan 31 '19 at 01:37
When you `goto EndofLoop1`, you write "pass", but then keep executing past the `EndofLoop2` label, and write "fail". — Blorgbeard, Jan 31 '19 at 01:39
The other problem is that your first label falls through into the second one. Now you need a third label after the second one, and a `goto` after the first one, so you skip the second if result is "pass". Note that your loops are not needed, since you're only ever comparing the first items.It's not clear what you *want* the code to do. Why are you using loops if you have a `goto` after the first comparison? — Rufus L, Jan 31 '19 at 01:41
your syntax isn't work for your condition, after goto EndOfLoop1 is hit. the program will traverse to the EndOfLoop1, and execute every line of code after it (including EndOfLoop2) — SKLTFZ, Jan 31 '19 at 01:41
you should just write a function to call for EndOfLoop and it should be outside your main route. — SKLTFZ, Jan 31 '19 at 01:43
@Emily yes, it should work too, just make sure your break is within your if statement (currently it break out the iteration even through the condition doesn't match) — SKLTFZ, Jan 31 '19 at 01:45
I have tried use break with in each if statement, it's failed to break the loop as well. That's why I backed to goto statement.. — HCYH, Jan 31 '19 at 01:46
as you are trying to break 2 for loop, thus one break cannot break out the outer loop, there are many ways to solve it, please try to reference https://stackoverflow.com/questions/9695902/how-to-break-out-of-nested-loops — SKLTFZ, Jan 31 '19 at 01:48

Joel Coehoorn · Answer 1 · 2019-01-31T02:33:13.687

There's no way the code in the question will ever look at more than the first cell.

The only difference between the two if() conditions is one uses == while the other uses !=; one of those conditions will always be true. Thus the very first cell will always break out of the loop. It's the same as if you had written this code, with no loop at all:

if (baseline[1, 1]?.ToString() == result_PriceUpdate[1, 1]?.ToString())
    goto EndofLoop1; 
if (baseline[1, 1]?.ToString() != result_PriceUpdate[1, 1]?.ToString()) 
    goto EndofLoop2;

EndofLoop1:
result_file.WriteResultToExcel("Result Summary", 2, 2, "pass");

EndofLoop2:
result_file.WriteResultToExcel("Result Summary", 2, 2, "fail");

Maybe you're relying on the ?. null conditional operator to create comparisons with null, with the idea this will behave the same as it does in SQL, where comparing a value with null could produce false for both the != and == conditions. That won't happen here.

Worse, the jump to EndOfLoop1 doesn't end the code. The EndOfLoop2 section is still part of the method, and it will also run. Once the second section runs, it replaces the work from the first section. Even when you pass, the result you'll see in the file is still "fail".

More than that, if somehow no condition in the loop is ever true, both named sections would still run after the loop finished.

Better practice here is simply don't use goto. There's no need, and it clearly confused things. Instead, set a string variable to either "pass" or "fail" and change i and j to int.MaxValue so the loop exits right away naturally. Then only have one WriteResultToExcel() to write out the string variable.

The obvious mistake with the loop means the code in the question is unfortunately not clear enough to determine your real intent. I will suggest a solution based on the idea you want to pass only if all cells pass, and if any cells fails that will fail the entire data set:

string result = "pass";
for (i = 0; i < baseline.GetLength(0); i++)
    for (j = 0; j < baseline.GetLength(1); j++)
        {
           if (baseline[i, j]?.ToString() != result_PriceUpdate[i, j]?.ToString()) 
           {
               result = "fail";
               i = int.MaxValue;
               j = int.MaxValue;
           }
        }
result_file.WriteResultToExcel("Result Summary", 2, 2, result);

I also have a question about starting with 1, rather than 0. C# arrays start at 0, but you never try to check the 0 positions for either dimension of the arrays, and this seems like a clear mistake.

Thanks @Joel Coehoorn But I got an error "Index was outside the bounds of the array." for this — HCYH, Jan 31 '19 at 02:31
@Emily You probably want to start at 0 and go while the loop variables are `<` the bounds, rather than `<=`. I've edited that change into my answer. — Joel Coehoorn, Jan 31 '19 at 02:32
@Emily then `baseline` is bigger than `result_PriceUpdate` (more rows?). — Nikola Markovinović, Jan 31 '19 at 02:37
This might be a good time to be extra sure that's really true. Do you get the same results for `GetLength()` for both dimensions of both arrays? — Joel Coehoorn, Jan 31 '19 at 02:46
And I'm not sure why it only compare one time which is baseline [1,1], then it gets break — HCYH, Jan 31 '19 at 02:59
If you don't understand that, it may be a good idea to spend more time with a 101 level textbook. — Joel Coehoorn, Jan 31 '19 at 03:17

score 0 · Answer 2 · edited Mar 19 '19 at 22:30

0

Finally I got the issue fixed. The loop was incorrect:

for (i = 1; i <= baseline.GetLength(0); i++)
    for (j = 1; j <= baseline.GetLength(1); j++)

It should be :

for (i = 1; i < baseline.GetLength(0); ++i)
    for (j = 1; j < baseline.GetLength(1); ++j)

edited Mar 19 '19 at 22:30

Wai Ha Lee

8,598
83
57
92

answered Mar 19 '19 at 21:57

HCYH

47
1
6

C# - Can't get the correct result using EndofLoop

2 Answers2