Condition of if/else in C# not behaving as expected

Question

I'm experiencing weird behavior in an if/else statement written in C#. I am new to C# and a novice with ASP.NET using Visual Studio Pro 2017.

I am working on adding functionality to a web app to allow for uploading of SQL data to table via user uploading a .CSV per this article by Mudassar Khan

All works well with this. I am now working to validate the data before the upload takes place.I am using the same .CSV from the article above but with a few "error" conditions introduced.

1,John,Hammond,United States
2,Shouldbreak,India
3,JohnMathews,Ireland
4,Robert,Schidner,Russia,sdfsdfdf,sdfsdfsdf,

In the code below you will see that I am testing to be sure there are the correct number of columns in each row by setting the required number of columns in a variable and comparing that to the number of commas found in each row present in the .CSV.

My desired validation is as follows:

If there are a correct number of columns add to dataTable.
If there are not the correct amount of columns.
1. Display ASP control which contains the error message.
2. If there are too many columns.
  1. Add a string to the error message that include the row number and the incorrect number of columns.
3. If there are too few columns.
  1. Add a string to the error message variable that includes the row number and the incorrect number of columns.

I am also tracking and displaying the number of commas found in each row in a separate ASP control for debugging purposes.

Below is the code in my code-behind file:

//Upload and save the file
string csvPath = Server.MapPath("~/_uploadedDataCSV/") + Path.GetFileName(FileUpload1.PostedFile.FileName);
FileUpload1.SaveAs(csvPath);

DataTable dt = new DataTable();

int columnCount = 4; // Expected number of columns
int commas = 0; // Set comma count to 0;
int tableRow = 0; // set current table row to 0;
Count.Text = string.Empty;
string errMessage = string.Empty;

dt.Columns.AddRange(new DataColumn[4] // Add expected number of columns to table
{
    // Add column Header and expected (c#)datatype 
    new DataColumn("Id", typeof(int)),
    new DataColumn("First Name", typeof(string)),
    new DataColumn("Last Name", typeof(string)),
    new DataColumn("Country",typeof(string))
});

string csvData = File.ReadAllText(csvPath); // Creates String of all content in CSV file
foreach (string row in csvData.Split('\n')) // Do the following for each row in the file
{
    // Start Validation Logic
    // - - - - - - - - - - - - - - - - - - - -

    tableRow++;

    // Count commas in row
    foreach (char ch in row)
    {
        if (ch.Equals(','))
        {
            commas++;
        }
    }

    if (!string.IsNullOrEmpty(row)) // if the row is not empty
    {
        if (commas == columnCount) // if commas in row equals the number of columns expected
        {
            dt.Rows.Add();
            int i = 0;
            foreach (string cell in row.Split(','))
            {
                dt.Rows[dt.Rows.Count - 1][i] = cell;
                i++;
            }
        }
        //Begin error reporting for row
        // - - - - - - - - - - - - - - - - - - - -
        else if (commas != columnCount)
        {
            err.Visible = true; // show error message element
            err.Text = errMessage; // set text of error message element to value of errMessage variable
            if (commas >= columnCount) // if too many columns in row
            {
                errMessage+= $"Row {tableRow} has too many columns <br />";
            }
            else if (commas < columnCount) // if not enough columns in row
            {
                errMessage+= $"Row {tableRow} does not have enough columns <br />";
            }
        }
    }

    Count.Text+= commas.ToString() + "|";
    commas = 0;
}

The issue I am having is when there are too many columns in a row, I am not getting the error message associated with this condition. The debug controller properly displays the number of commas and if there are too few, displays the message properly, just not the message for too many. When I step through the code in debugger the variables (commas / columnCount) are reported corrected for all the rows.

Any suggestions? I am happy to provide more information if I left anything out.

I have tried removing the condition of too few columns and it still doesn't display the message for too many columns. I have also tried reversing the order of the check but the same output is given. I have tried rebuilding per this discussion.

Please try to reproduce this in a [mcve]. It's very unlikely that you can only reproduce this in ASP.NET. Try to reproduce it in a console app that does *nothing* but demonstrate the problem. (So for example, I doubt that you actually need the `DataTable` here.) At the same time, make it complete enough that we can copy/paste/compile/run. Ideally, hard-code the data (again, just enough to demonstrate the problem). — Jon Skeet, Mar 25 '18 at 16:51
I think this is simple: you're setting `err.Text = errMessage;` **before** appending to the `errMessage` variable. So `err.Text` has the empty string; or rather it will have whatever it had in the last iteration, since you're also doing this inside a loop, and setting `err.Text` within that loop. Instead, set it after the loop has completed. — Heretic Monkey, Mar 25 '18 at 16:54
if you split your line before the logic to insert the data in the table then you don't need that loop over every character to find the commas — Steve, Mar 25 '18 at 16:57
Tip unrelated to the problem: By definition, the `else` clause for `if (commas == columnCount)` will be when `commas != columnCount` (there is no other possibility), so `else if (commas != columnCount)` becomes just `else`. — Richardissimo, Mar 25 '18 at 17:07
Tip unrelated to the problem: Rather than `File.ReadAllText`, and then using `Split('\n')`, [prefer](https://stackoverflow.com/a/21970035/5198140) `File.ReadLines` which will give you a collection of strings, one for each line, which you can still use in your `foreach`, but it won't hold the whole file in memory at once. — Richardissimo, Mar 25 '18 at 17:12
Tip unrelated to the problem: rather than `dt.Rows[dt.Rows.Count - 1]`, store the result of `dt.Rows.Add()` in a variable, and use that: it saves you from looking up the Rows property of the dt variable, then calculating count minus one (which involves going via the dt variable's Rows property, to get the count), and then using an indexer to look up the row; and you're doing that inside a loop, so it will do it for each cell. Actually, you might even be able to ditch all of that foreach loop, and just do `dt.Rows.Add(row.Split(','));` — Richardissimo, Mar 25 '18 at 17:18
Tip unrelated to the problem: similar to 3 tips earlier: `else if (commas < columnCount)` can be just `else`. — Richardissimo, Mar 25 '18 at 17:21
Tip unrelated to the problem: when concatenating a lot of strings line this, [it is advised](https://stackoverflow.com/questions/73883/string-vs-stringbuilder) to use a `System.Text.StringBuilder`, rather than just doing `+=` to construct the string. — Richardissimo, Mar 25 '18 at 17:25
If you're going to split the row on the `','` character anyway, you don't need to count the commas beforehand. Just do something like `var cells = row.Split(',');` then you can use `int commas = cells.Length - 1` to get the number of commas in the `row`. — Rufus L, Mar 25 '18 at 17:50
@MikeMcCaughan This answer was right on the money. Thanks so much. Not sure I completely understand why (even after reading @EduardAbliazimov answer). I moved the `err.Text = errMessage;` to after the close of the `else` and it is working as expected now. — djlotus, Mar 25 '18 at 19:36
@Richardissimo I will take these tips. I am just learning C# as I said, and anything that helps write cleaner more efficient code is always appreciated. — djlotus, Mar 25 '18 at 19:38

score 1 · Accepted Answer · answered Mar 25 '18 at 17:18

All credits should go to @Mike McCaughan, his comment is absolutely in place. I just want to give a little example as of what was stated by Mike:

string a = string.Empty;
string b = a;
a += "3";
Console.WriteLine(b); //result in empty string in console

back to your problem:

string errMessage = string.Empty;
err.Text = errMessage;
errMessage+= $"Row {tableRow} has too many columns <br />";
Console.WriteLine(err.Text); //result in empty string in console

The reason why it happens is because of the immutable nature of string in c#. Means any changes to the string object will result in a new object creation. You can find more information there: Immutability of String Objects

Condition of if/else in C# not behaving as expected

1 Answers1