Link to home
Start Free TrialLog in
Avatar of juststeve
juststeve

asked on

Values in a list of objects seem to be altered after 'Add'

I'm troubleshooting some strangeness with how values are stored in a collection of objects as they iterate thru a foreach loop.

I have a batch of orders generated by emails (as opposed to form-based order entry where validation is in real time). Some of these orders are well formed and can be entered to the system as is.

I'm trying to handle those with bad data separately from well formed orders with the loop pasted below. If an order is not well formed I need to reach back to the original email and inject that string into the order (provides a clue to the reviewer as to what the order's intent was).

                foreach (var row in rows)
                {
                    if (row.Book.ID == 0)  //ambiguous order
                    {
                        rawOrder = GetRawOrder(row.ID); // retrieves original string from email
                        row.Book.Title = rawOrder["BookTitle"]; // injects that string
                        unknownTitles.Add(row);
                    }
                    else
                    {
                        validTitles.Add(row);
                    }
                }

I'm expecting to see a collection of orders something like:
[Title: 'Old Yellow' | User: Bob]
[Title: 'War and Piece' | User: Mary]
[Title: 'Valley of the Dales' | User: Frank]

But instead:
[Title: 'Old Yellow' | User: Bob]
[Title: 'Old Yellow' | User: Mary]
[Title: 'Old Yellow' | User: Frank]


If I single-step my way thru the loop I'm seeing the correct info at the first iteration:

>?  rawOrder["BookTitle"] // Old Yellow
>? unknownTitles[0]Book.Title // Old Yellow
>? unknownTitles[0].User // Bob

but on the second iteration if I re-examine the same order I see different values.
>?  rawOrder["BookTitle"] // War and Piece : that'd be correct - it's the 2nd iteration
>? unknownTitles[0].Title // War and Piece : wrong - why would this change
>? unknownTitles[0].User // Bob : This value _didn't change

It's not a matter of the .Add injecting each new row at the '0' position or the user name would also have changed.

Normally when I do something this obviously wrong, I've figured it out before i submit the question but here I'm just not seeing it.


Avatar of Jacques Bourgeois (James Burger)
Jacques Bourgeois (James Burger)
Flag of Canada image

Could it be that row.Book.ID is always 0 for all the books? You would then change all the books to the same one.
Avatar of juststeve
juststeve

ASKER

I've triple checked - the 'Title' value for all the records are being changed at each iteration. Here's what the values look like after 4 iterations:

>? unknownTitles[0].Book.Title
" Old Yellow "
>? unknownTitles[1].Book.Title
" Old Yellow "
>? unknownTitles[2].Book.Title
" Old Yellow "
>? unknownTitles[3].Book.Title
" Old Yellow "

Then I let the loop iterate one more time and look at those same records.
>? unknownTitles[0].Book.Title
" War and Piece "
>? unknownTitles[1].Book.Title
" War and Piece "
>? unknownTitles[2].Book.Title
" War and Piece "
>? unknownTitles[3].Book.Title
" War and Piece "
>? unknownTitles[4].Book.Title
" War and Piece "

The 'Name' value is not changed - every other aspect of the code it producing correct output.

I've been starring at this code for hours - just don't see how it can be producing the results it does. This one's really bugging me.

                    if (row.Book.ID == 0)  //ambiguous order
                    {
                        rawOrder = GetRawOrder(row.ID); // retrieves original string from email
                        row.Book.Title = rawOrder["BookTitle"]; // injects that string
                        unknownTitles.Add(row);
                    }

As I asked before, have you checked the ID of all the books before going into the loop? This could be the cause of your problem, because the if would execute for all the books, GetRawOrder would always return the same book, and assign the Title of that book to all the others.
I've probably confused you be not making it clear that I'm producing 2 different collections of orders:
            IList<OrderRow> validTitles = new List<OrderRow> { };
            IList<OrderRow> unknownTitles = new List<OrderRow> { };

The foreach loops thru all books and Adds each to the given collection depending on the value of the Book.ID. (IOW, the foreach is the means of 'checking all Book IDs'.)

Valid orders are merely added to the 'validTitles' collection. If invalid, I'm fetching the Raw string and saving it on a row-by-row basis.

So when I overwrite the title (row.Book.Title = rawOrder["BookTitle"];)  I'm expecting to only effect the current iteration - i don't see how that line of code can be effecting rows that have already been stored to the collection.
and to clarify:

rawOrder = GetRawOrder(row.ID);

is stepping back the individual email the spawned the order. Since each row is matching up to a discrete email there's no good reason for a 'all the same title' condition.
I am sorry to insist, but have you checked the actual value returned by row.ID. Something, what we think happens is not what happens.

Do you have the code for GetRawOrder?
Yes...row.ID changes with the iterations. And rawOrder["BookTitle"]; returns different values as expected. I've put eyes on that.

All values - all properties - all methods work as expected except this:

   row.Book.Title = rawOrder["BookTitle"];

This statement is changing, not just the current instance of row, its also changing all the instances that have already been stored to the collection (as shown by the above pasted output).

I'm expecting that instances of row that have already been stored to the collection to be untouched by any future iterations but clearly, they are being updated.

   >? row.ID //420932
   >? rawOrder["BookTitle"] //"War and Piece"
   >? unknownTitles[0].Book.Title //"War and Piece"

Correct. Now iterate.

   >? row.ID //420933
   >? rawOrder["BookTitle"] //"Old Yellow"
   >? unknownTitles[1].Book.Title //"Old Yellow"

Also correct. But now look at the value in the first instance.

>? unknownTitles[0].Book.Title
"Old Yellow" //shoud be "War and Piece"


shouldn't it?
SOLUTION
Avatar of jagrut_patel
jagrut_patel
Flag of India image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
OK, now that the ID is cleared, next step.

It might be that all the rows point to a unique object. Object variables are pointers. If you reuse the same variable to fill each rows, you might end up with many rows that display the same object. Changing one will change the others.

How is the rows collection or array built?
Thankx - that suggestion shows me the problem is, indeed, somewhere else in the app. Now can you tell me how I set a breakpoint that will stop execution at the point where value (unknownTitles[0].Book.Title) is changed?

thx
Sorry James - my last comment was on a non-refreshed page so I didn't see your question...

How is the unknownTitles constructed?

Undoubtedly you are pinpointing my error but even with my nose this close to it...

IList<OrderRow> validTitles = new List<OrderRow> { };
IList<OrderRow> unknownTitles = new List<OrderRow> { };
      
IList<OrderRow> rows = OrderFacade.Instance.GetOrdersAwaitingVerification();
var rawOrder = new Dictionary<string, string> {};

foreach (var row in rows)
{
    if (row.Books.ID == 386)
    {
        rawOrder = GetRawOrder(row.ID);
        row.Books.Title = rawOrder["BooksTitle"];
        unknownTitles.Add(row);
    }
    else
    {
        validTitles.Add(row);
    }
}
var model = new ImportOrdersViewModel
{
    Unknown = unknownTitles,
    Imported = validTitles
};
To set a breakpoint, click in the grey margin at the left of the line you want to break on.

It's the code that build rows that I see as a possible culprit at this point. I would thus need to see GetOrdersAwaitingVerification.
re: breakpoint - breakpoint in margin is how i've been collecting the info i've been pasting. Since the value change is taking place somewhere else, I'm looking to break execution at the exact point when the value changes.

Here's GetRawOrder:

public Dictionary<string, string> GetRawOrder(int orderRowID)
{
    OrderRow row = OrderFacade.Instance.LoadOrderRow(orderRowID);
    var connSproc = new SqlConnection(ConfigurationManager.AppSettings["dbConnection"]);
    connSproc.Open();

    string getRecordID = row.Order.AuditInfo.ToString().Split(new string[] { "#RecordID#" }, StringSplitOptions.RemoveEmptyEntries)[1].ToString();
    getRecordID = getRecordID.Replace(" ", "");
    int recordID = Convert.ToInt32(getRecordID);

    var cmdGetBody = new SqlCommand(
        "Select ID, emailID, emailBody, EmailDate, orderStatus" +
        "  from IncomingEmailOrders where id =  " + recordID, connSproc) {CommandType = CommandType.Text};
    // execute the command
    SqlDataReader msgReader = cmdGetBody.ExecuteReader();
    var emailMsg = "";
    while (msgReader.Read())
    {
        emailMsg = msgReader[2].ToString();
        if (recordID < 0)
        {
            break;
        }
    }
    connSproc.Close();

    var values = OrderGateway.Instance.ParseToDict(emailMsg);
    return values;
}
geeze....too many balls in the air this week...

public virtual IList<OrderRow> GetOrdersAwaitingVerification()
        {
            Query q = new Query();
            q.Criteria.Add(new Criteria("Status", CriteriaOperator.Equal, OrderRowStatus.AwaitingVerification));
            q.OrderClauses.Add(new OrderClause("Webinar.Date", OrderClauseCriteria.Ascending));
            return DataContext.LoadList<OrderRow>(q);
           
}
Cannot see what is happening either. You are using a Query object that seems to be some class you have created in your toolbox. I have never encountered a class of that name in the framework, and the only one I see in the documentation deals with DirectX.Direct3D.

So I still do not see how the rows collection in the original posting is built.

This is going too deep and would need to be followed step by step in order to see what is going on.

jagrut_patel has suggested the following code in a previous post. Have you tried it to see what is in the collection before entering the loop. That could give a clue:
foreach (var row in rows)
{
   Console.Writeline(row.Book.ID + "," + row.Book.Title + "," + row.Book.User);
}

Open in new window

It's an nHibernate query construction. But, honestly, I think the means of construction of the original collection is immaterial. It's just an IList collection of my OrderRow object. Nothing exotic. I've been using variations of this OrderRow object (and nHibernate-produced collections of it) for a few years in a production site.

The suggested code snippet really just verified that unique strings were being stored at each iteration. I've modified the idea by writing the current state of several of the relavent value from within the effected loop. The primary goal is to look at the value of the 1st instance being stored in the collection of orders.


if (row.Book.ID == 386)
{
      var storedTitle = (unknownTitles.Count > 0 ? unknownTitles[0].Book.Title : "nothing there yet"); // accomodates the null value on 1st run
      holder += myIterator.ToString() + ". Before GetRaw: " + storedTitle + Environment.NewLine;
      var rawOrder = GetRawOrder(row.ID);
      holder += (myIterator.ToString() + ". Before assign Title: "  + rawOrder["BookTitle"] + ", " + storedTitle + ", " + row.Order.User.ID) + Environment.NewLine;  
      row.Book.Title = rawOrder["BookTitle"];
      holder += (myIterator.ToString() + ". After assign Title: " + rawOrder["BookTitle"] + ", " + storedTitle + ", " + row.Order.User.ID) + Environment.NewLine;
      unknownTitles.Add(row);
      holder += (myIterator.ToString() + ". after Add(Row): "  + rawOrder["BookTitle"] + ", " + unknownTitles[0].Book.Title + ", " + unknownTitles[0].Order.User.ID) + Environment.NewLine;  
      
      myIterator++;
}

This output establishes that the 1st instance's Title property is being changed at the point where the 2nd instance is being added to the collection. It also shows that _only the Title property changes - the User's ID remains intact.

Here's the value of 'holder' after 2 iterations.
0. Before GetRaw: nothing there yet
0. Before assign Title: War and Piece, nothing there yet, 9195
0. After assign Title: War and Piece, nothing there yet, 9195
0. after Add(Row): War and Piece, War and Piece, 9195
1. Before GetRaw: War and Piece
1. Before assign Title: Old Yellow, War and Piece, 980
1. After assign Title: Old Yellow, War and Piece, 980
// correct - the newly retrieved RawOrder value is 'Old Yellow' but the original Title value is still intact. Now we add the row to the collection and see
1. after Add(Row): Old Yellow, Old Yellow, 9195
// ... that the original Title value is changed but the User's ID hasn't.

What I let looping continue I see that the Title value for _all previous instances are being changed to the current. Again....UserID (and all other object properties) are correctly retained.

In a previous message you talked about 'object variables as pointers' and I'd thought that'd be the root of the problem. But I've moved the instantiation of GetRawOrder to be fully within the loop without any change in behavior. wow
I still don't see what is happening under the assignation.

Concentrating on the if (row.Book.ID == 386) is useless. This is where the problem is detected, but the problem lies under the assignation and I do not see enough of what is there.

If you put a breakpoint on row.Books.Title = rawOrder["BooksTitle"]; and go step by step with Step Into (F11) do you go into code that you can see. The problems lies there somewhere, not in the assignation itself.

F11 goes to the opening braces of the getter for book. 2nd f11 get 'return mBook' - the 3rd to the closing brace. Repeats the same three steps for Title.


        public Book Book
        {
            get { return mBook; }
            set { mBook = value; }
        }

and
                public string Title
        {
            get { return mTitle; }
            set { mTitle = value; }
        }


So this is the 'by reference' issue. I'm incorrect in thinking that i'm storing a series of object that are disconnected one from another. Even though rawOrder is being instantiated each iteration, it still has a reference to values stored in prior iterations.

Any way to break that chain?
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Thankx - i think i've chased this puppy as far as I can for now and am ready to write it off to yarhol (yet another rabbit hole of learning). Sure would have been nice to have had better granularity of breakpoints - a way to say 'stop everything when this instance's property changes' cuz to my eye there's nothing is happening inbetween :
      row.Book.Title = rawOrder["BookTitle"];
and
      unknownTitles.Add(row);

Though bad on me for not using F11 sooner. In any event, this rabbit hole has revealed all that I care to explore for now - workarounds abound.

thx for your persistence.
Thankx for stepping in jagrut...appreciated