0

UPDATE
I think I have found what is causing the issue here https://stackoverflow.com/a/5665600/19393524
I believe my issue lies with my use of .DefaultView. The post thinks when you do a sort on it it is technically a write operation to the DataTable object and might not propagate changes made properly or entirely. It is an interesting read and seems to answer my question of why passing valid data to a DataRow is throwing this exception AFTER I make changes to the datatable

UPDATE: Let me be crystal clear. I have already solved my problem. I would just like to know why it is throwing an error. In my view the code should work and it does.. the first run through.

AFTER I have already deleted the column then added it back (run this code once)

When I debug my code line by line in Visiual studio and stop at the line: data.Rows[i].SetField(sortColumnNames[k], value);

  • the row exists
  • the column exisits
  • value is not null
  • sortColumnNames[k] is not null and contains the correct column name
  • i is 0
    Yet it still throws an exception. I would like to know why. What am I missing?

Sorry for the long explanation but this one needs some context unfortunately.

So my problem is this, I have code that sorts data in a DataTable object by column. The user picks the column they want to sort by and then my code sorts it.

I ran into an issue where I needed numbers to sort as numbers not strings (all data in the table is strings). eg (string sorting would result in 1000 coming before 500)

So my solution was to create a temporary column that uses the correct datatype so that numbers get sorted properly and the original string data of the number remains unchanged but is now sorted properly. This worked perfectly. I could sort string numeric data as numeric data without changing the formatting of the number or data type.

I delete the column I used to sort afterwards because I use defaultview to sort and copy data to another DataTable object.

That part all works fine the first time.

The issue is when the user needs to do a different sort on the same column. My code adds back the column. (same name) then tries to add values to the column but then I get a null reference exception "Object not set to an instance of an object"

Here is what I've tried:

  • I've tried using AcceptChanges() after deleting a column but this did nothing.
  • I've tried using column index, name, and column object returned by DataTable.Columns.Add() in the first parameter of SetField() in case it was somehow referencing the "old" column object I deleted (this is what I think the problem is more than likely)
  • I've tried changing the value of the .ItemArray[] directly but this does not work even the first time

Here is the code:

This is the how the column names are passed:

private void SortByColumn()
        {
            if (cbAscDesc.SelectedIndex != -1)//if the user has selected ASC or DESC order
            {
                //clears the datatable object that stores the sorted defaultview
                sortedData.Clear();

                //grabs column names the user has selected to sort by and copies them to a string[]
                string[] lbItems = new string[lbColumnsToSortBy.Items.Count];
                lbColumnsToSortBy.Items.CopyTo(lbItems, 0);

                //adds temp columns to data to sort numerical strings properly
                string[] itemsToSort = AddSortColumns(lbItems);         
                
                //creates parameters for defaultview sort
                string columnsToSortBy = String.Join(",", itemsToSort);
                string sortDirection = cbAscDesc.SelectedItem.ToString();
                data.DefaultView.Sort = columnsToSortBy + " " + sortDirection;

                //copies the defaultview to the sorted table object
                sortedData = data.DefaultView.ToTable();
                RemoveSortColumns(itemsToSort);//removes temp sorting columns
            }
        }

This is where the temp columns are added:

 private string[] AddSortColumns(string[] items)//adds columns to data that will be used to sort
                                                       //(ensures numbers are sorted as numbers and strings are sorted as strings)
        {
            string[] sortColumnNames = new string[items.Length];
            for (int k = 0; k < items.Length; k++)
            {
                int indexOfOrginialColumn = Array.IndexOf(columns, items[k]);
                Type datatype = CheckDataType(indexOfOrginialColumn);
                if (datatype == typeof(double))
                {
                    sortColumnNames[k] = items[k] + "Sort";

                    data.Columns.Add(sortColumnNames[k], typeof(double)); 

                    for (int i = 0; i < data.Rows.Count; i++)
                    {
                        //these three lines add the values in the original column to the column used to sort formated to the proper datatype
                        NumberStyles styles = NumberStyles.Any;
                        double value = double.Parse(data.Rows[i].Field<string>(indexOfOrginialColumn), styles);
                        bool test = data.Columns.Contains("QtySort");
                        data.Rows[i].SetField(sortColumnNames[k], value);//this is line that throws a null ref exception
                    }
                }
                else
                {
                    sortColumnNames[k] = items[k];
                }
            }
            return sortColumnNames;
        }

This is the code that deletes the columns afterward:

private void RemoveSortColumns(string[] columnsToRemove)
        {
            for (int i = 0; i < columnsToRemove.Length; i++)
            {
                if (columnsToRemove[i].Contains("Sort"))
                {
                    sortedData.Columns.Remove(columnsToRemove[i]); 
                }
            }
        }

NOTE: I've been able to fix the problem by just keeping the column in data and just deleting the column from sortedData as I use .Clear() on the sorted table which seems to ensure the exception is not thrown.

I would still like an answer though as to why this is throwing an exception. If I use .Contains() on the line right before the one where the exception is thrown is says the column exists and returns true and in case anyone is wondering the params sortColumnNames[k] and value are never null either.

Kotorfreak
  • 11
  • 6
  • 2
    You are changing the collection `data.Columns` while enumerating through it, which is always a big no-no. First get what you want to remove, store it separately, then remove it (or, in this case, as you've found out, just don't change the collection because it'll happen later anyway). – Jeroen Mostert Jun 29 '22 at 14:12
  • @JeroenMostert I'm not sure which part of my code you are talking about I am still learning. I remove the column in a helper method after I have already sorted the data and copied the sort to `sortedData`, also I works just fine the first time the code is run. It is only after when the code is run a second time that the exception is thrown. Could you explain where in my code I am enumerating through `data.Columns` while also changing it? That would help me avoid this mistake in the future. – Kotorfreak Jun 29 '22 at 14:30
  • 1
    I'm only talking about `RemoveSortColumns`, which contains an obvious error (removing a column in the middle desynchronizes your `i` counter from the original collection). The rest of your code I've admittedly not bothered to dig through deeply, though it seems very likely the error there is causing the rest to fail. The pattern ``for (...; i < collection.Count; ...) { /* change the number of items in `collection` in some way */ }`` is basically always wrong, with the best case being getting an error immediately, and the worst case inexplicable failures later. – Jeroen Mostert Jun 29 '22 at 14:34
  • 2
    You state that… _”I ran into an issue where I needed numbers to sort as numbers not strings (all data in the table is strings). eg (string sorting would result in 1000 coming before 500)”_ … It sounds to me like it would be well worth the effort to store the numbers as actual “numbers” and NOT `strings`. Storing numbers as `strings` is never a good idea and is only guaranteed to create extra work for you as your question demonstrates. Stop creating unnecessary work for yourself and store the numbers properly as numbers. – JohnG Jun 29 '22 at 15:15
  • @JohnG I understand that. All data I work with comes from .txt and .csv files. I would do that if it did not change how the number was formatted and displayed. eg (0000090 gets changed to 90). I need to keep this original formatting. I could just reformat the number back to the string afterward but then I would need count the leading zeros before conversion and store how many there were before hand then add them back again when they eventually get exported to .txt/.csv file. This would make my code more complex which is what I am trying to avoid. – Kotorfreak Jun 29 '22 at 15:34
  • Going back to the original problem for a fresh rethink as @JohnG points out you should: another option is to use a Natural Sort string comparer, such as this one https://stackoverflow.com/a/248613/14868997 which will sort numbers within strings nicely – Charlieface Jun 29 '22 at 15:52
  • Obviously, I am missing something. I have to ask… why would you store 90 as 00000090? Do the prefixed 0 digits represent something? Sorry I am just not understanding what the 0 prefix digits represent. – JohnG Jun 29 '22 at 15:55
  • @JohnG the number in question is a sequence number that is printed later on a surface. The leading zeros must be printed as well because other utilities that we use rely on the number being a fixed number of digits to process the data further later. – Kotorfreak Jun 29 '22 at 16:01
  • _”The leading zeros must be printed as well because other utilities that we use rely on the number being a fixed number of digits to process the data further later.”_ … ? … If the number needs to be a FIXED number of digits “later” by some other utility, then would it not be easier to “pad” the number in THAT utility as opposed to maintaining this padded fixed size number when saving the number in the text file? – JohnG Jun 29 '22 at 16:24
  • This sounds like you are putting the cart before the horse. Store the number properly to avoid what you are having to go through right NOW as your question demonstrates and other places in your code that needs the NUMBER. PAD the number to the proper number of digits in the other utilities. Your approach is guaranteeing that IF the code needs the number, then you are forced to write extra and unnecessary code. Sorry I will drop this if you want to go down that rabbit hole. Good Luck. – JohnG Jun 29 '22 at 16:24
  • @JohnG I appreciate the help you've given. Unfortunately changing the source code in the other utilities is not an option as that could introduce bugs in otherwise working code and if those utilities break I'm in trouble and it would cost my company money. This utility I am working on is just a side project I am working on and is not a high priority. I just was curious why code that looks like should work doesn't. I've updated my post with a more specific question and more details from my debugger. Again I really appreciate your feedback! Hope you have a nice day :) – Kotorfreak Jun 29 '22 at 18:09
  • Are ALL the numbers the same “fixed” size? If so, then it should be trivial to PAD the number to that fixed size when you write it to the file. I can certainly understand not being able to change the utility code, however, it MUST need the text file you are writing to. So, it seems odd to me to “maintain” this odd format when YOUR code runs. For YOUR code, make it a number, run it and when you write it back to the file… then format it any way you want. – JohnG Jun 29 '22 at 18:49
  • It just seems odd to me, that you want to “maintain” this number format in YOUR code and thus creating all this extra code you need to implement to get it to work as a number… Like adding a column to the table then deleting it. IMO, it would simplify things greatly if YOUR code simply dropped the format and used it as a number and formatted it when you write it to the file. – JohnG Jun 29 '22 at 18:50
  • I agree with you completely. It's just not ideal for me to change the source of the other utilities as we implement hundreds of different ones for different clients with different kinds of source files and data types. Another issue is there is no set coding standard where I work and other source codes are either hard to read/messy or we don't have the source code at all because it was developed locally on an old programmers machine and then never uploaded to a repository. It's kind of a mess here haha. – Kotorfreak Jun 29 '22 at 18:57
  • I understand, but I think you are missing my point… forget about all the other utilities and “other” messy parameters. Think strictly in reference to YOUR current code. In your code, it does NOT need the number formatted the way it is stored in the file… so just because it is stored that way doesn’t mean YOU have to use it that way. The key is writing it back to the file where you WOULD need to use the formatted number. It makes no sense for YOUR code to jump through all these hoops to “maintain” a format it does not need or use. – JohnG Jun 29 '22 at 19:46
  • I copy/paste your code and I have a few questions… What is `lbColumnsToSortBy` … ? … Where is `columns` defined… `int indexOfOrginialColumn = Array.IndexOf(columns, items[k]);` … ? … What does `CheckDataType` do? … `Type datatype = CheckDataType(indexOfOrginialColumn);` … ? … And why is the call to … `RemoveSortColumns();` … not have a `string[]` parameter? – JohnG Jun 29 '22 at 20:06
  • - `lbColumnsToSortBy` is a list box control that stores all the user selected sort columns - `indexOfOriginalColumn` is the originial index of the column in the data - `CheckDataType` uses `double.TryParse()` to see if EVERY string can be a number, returns `Type double` otherwise 'Type string' - `RemoveSortColumns()` I just forgot to update the code in the question after I changed it in my own code to avoid modifying the `.Columns` collection while looping through it – Kotorfreak Jun 30 '22 at 12:41
  • @JohnG - `columns` is defined with global scope. It holds the column names from the data in the proper order. See above comment for the rest of my explanation. SO is yelling at me for editing comments for too long. – Kotorfreak Jun 30 '22 at 12:51

2 Answers2

0

Your problem is probably here:

private void RemoveSortColumns()
        {
            for (int i = 0; i < data.Columns.Count; i++)
            {
                if (data.Columns[i].ColumnName.Contains("Sort"))
                {
                    data.Columns.RemoveAt(i);
                    sortedData.Columns.RemoveAt(i);
                }
            }
        }

If you have 2 columns, and the first one matches the if, you will never look at the second.

This is because it will run:

  • i = 0
  • is i < columns.Count which is 2 => yes
  • is col[0].Contains("sort") true => yes
  • remove col[0]
  • i = 1
  • is i < columns.Count which is 1 => no

The solution is to readjust i after the removal

private void RemoveSortColumns()
        {
            for (int i = 0; i < data.Columns.Count; i++)
            {
                if (data.Columns[i].ColumnName.Contains("Sort"))
                {
                    data.Columns.RemoveAt(i);
                    sortedData.Columns.RemoveAt(i);
                    i--;//removed 1 element, go back 1
                }
            }
        }
Cine
  • 4,255
  • 26
  • 46
  • I just tried adding the `i--` which admittedly was a mistake but that did still not resolve the issue. I still get the same null reference exception. I've updated my question code to keep people from pointing this out in the future – Kotorfreak Jun 29 '22 at 14:48
  • That is *a* solution, but it's not exactly obvious, and for some specialized collections this approach can easily fail because it assumes the relative order of items will not change -- which is true for a column collection, but not necessarily in general. A better pattern is to first find the columns to remove in one step, then remove all of them in the next step (i.e. `var columnsToRemove = (from DataColumn c in data.Columns where c.ColumnName.Contains("Sort") select c.ColumnName).ToList(); foreach (var columnName in columnsToRemove) { data.Columns.Remove(columnName); }`). – Jeroen Mostert Jun 29 '22 at 14:50
-1

I fixed my original issue by changing a few lines of code in my SortByColumn() method:

private void SortByColumn()
        {
            if (cbAscDesc.SelectedIndex != -1)//if the user has selected ASC or DESC order
            {
                //clears the datatable object that stores the sorted defaultview
                sortedData.Clear();

                //grabs column names the user has selected to sort by and copies them to a string[]
                string[] lbItems = new string[lbColumnsToSortBy.Items.Count];
                lbColumnsToSortBy.Items.CopyTo(lbItems, 0);

                //adds temp columns to data to sort numerical strings properly
                string[] itemsToSort = AddSortColumns(lbItems);         
                
                //creates parameters for defaultview sort
                string columnsToSortBy = String.Join(",", itemsToSort);
                string sortDirection = cbAscDesc.SelectedItem.ToString();
                DataView userSelectedSort = data.AsDataView();
                userSelectedSort.Sort = columnsToSortBy + " " + sortDirection;

                //copies the defaultview to the sorted table object
                sortedData = userSelectedSort.ToTable();
                RemoveSortColumns(itemsToSort);//removes temp sorting columns
            }
        }

Instead of sorting on data.DefaultView I create a new DataView object and pass data.AsDataView() as it's value then sort on that. Completely gets rid of the issue in my original code. For anyone wondering I still believe it is bug with .DefaultView in the .NET framework that Microsoft will probably never fix. I hope this will help someone with a similar issue in the future.
Here is the link again to where I figured out a solution to my problem.
https://stackoverflow.com/a/5665600

Kotorfreak
  • 11
  • 6