Trouble with EF "Save" method, modified collection of child entities

Question

I have a parent entity (Treatment) with a collection of child entities (Segments). I have a save method that take a treatment, determines if it's new or existing, and then either adds it to the objectContext or attaches it to the object context based on whether it is new or existing.

It does the same thing with the children within the main entity. It iterates over the collection of child entities, and then adds or updates as appropriate.

What I'm trying to get it to do, is to delete any child objects that are missing. The problem is, when I'm updating the parent object and then I attach it to the object context, the parent object then has a collection of child objects from the DB. Not the collection I originally passed in. So if I had a Treatment with 3 segments, and I remove one segment from the collection, and then pass the Treatment into my save method, as soon as the Treatment object is attached to the objectcontext, the number of segments it has is changed from 2 to 3.

What am I doing wrong?

Here is the code of my save method:

public bool Save(Treatment myTreatment, modelEntities myObjectContext)
        {
            bool result = false;

            if (myObjectContext != null)
            {
                if (myTreatment.Treatment_ID == 0)
                {
                    myObjectContext.Treatments.AddObject(myTreatment);
                }
                else
                {
                    if (myTreatment.EntityState == System.Data.EntityState.Detached)
                    {
                        myObjectContext.Treatments.Attach(myTreatment);
                    }
                    myObjectContext.ObjectStateManager.ChangeObjectState(myTreatment, System.Data.EntityState.Modified);
                    myObjectContext.Treatments.ApplyCurrentValues(myTreatment);
                }

                foreach (Segment mySegment in myTreatment.Segments)
                {
                    if (mySegment.SegmentID == 0)
                    {
                        myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Added);
                        myObjectContext.Segments.AddObject(mySegment);
                    }
                    else
                    {
                        if (mySegment.EntityState == System.Data.EntityState.Detached)
                        {
                            myObjectContext.Segments.Attach(mySegment);
                        }
                        myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Modified);
                        myObjectContext.Segments.ApplyCurrentValues(mySegment);
                    }
                }
            }

            result = (myObjectContext.SaveChanges(SaveOptions.None) != 0);


            return result;
        }

*EDIT**** Based on some of the feedback below, I have modified the "Save" method. The new method implementation is below. However, it still does not delete Segments that have been removed from the myTreatments.Segments collection.

public bool Save(Treatment myTreatment, tamcEntities myObjectContext)
        {
            bool result = false;

            if (myObjectContext != null)
            {
                if (myTreatment.Treatment_ID == 0)
                {
                    myObjectContext.Treatments.AddObject(myTreatment);
                }
                else
                {
                    if (myTreatment.EntityState == System.Data.EntityState.Detached)
                    {
                        myObjectContext.Treatments.Attach(myTreatment);
                    }
                    myObjectContext.ObjectStateManager.ChangeObjectState(myTreatment, System.Data.EntityState.Modified);
                }

                foreach (Segment mySegment in myTreatment.Segments)
                {
                    if (mySegment.SegmentID == 0)
                    {
                        myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Added);
                    }
                    else
                    {
                        myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Modified);
                    }
                }
            }

            result = (myObjectContext.SaveChanges(SaveOptions.None) != 0);


            return result;
        }

FINAL EDIT I have finally got it to work. Here is the updated Save method that is working properly. I had to save the initial list of Segments in a local variable and then compare it to the myTreatments.Segments list after it was attached to the DB, to determine a list of Segments to be deleted, and then iterate over that list and delete matching Segments from the newly attached myTreatment.Segments list. I also removed the passing in of the objectcontext per advice from several responders below.

public bool Save(Treatment myTreatment)
        {
            bool result = false;


            List<Segment> myTreatmentSegments = myTreatment.Segments.ToList<Segment>();

            using (tamcEntities myObjectContext = new tamcEntities())
            {
                if (myTreatment.Treatment_ID == 0)
                {
                    myObjectContext.Treatments.AddObject(myTreatment);
                }
                else
                {
                    if (myTreatment.EntityState == System.Data.EntityState.Detached)
                    {
                        myObjectContext.Treatments.Attach(myTreatment);
                    }
                    myObjectContext.ObjectStateManager.ChangeObjectState(myTreatment, System.Data.EntityState.Modified);
                }

                // Iterate over all the segments in myTreatment.Segments and update their EntityState to force
                // them to update in the DB.
                foreach (Segment mySegment in myTreatment.Segments)
                {
                    if (mySegment.SegmentID == 0)
                    {
                        myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Added);
                    }
                    else
                    {
                        myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Modified);
                    }
                }

                // Create list of "Deleted" segments
                List<Segment> myDeletedSegments = new List<Segment>();
                foreach (Segment mySegment in myTreatment.Segments)
                {
                    if (!myTreatmentSegments.Contains(mySegment))
                    {
                        myDeletedSegments.Add(mySegment);
                    }
                }
                // Iterate over list of "Deleted" segments and delete the matching segment from myTreatment.Segments
                foreach (Segment mySegment in myDeletedSegments)
                {
                    myObjectContext.ObjectStateManager.ChangeObjectState(mySegment, System.Data.EntityState.Deleted);
                }

                result = (myObjectContext.SaveChanges(SaveOptions.None) != 0);
            }
            return result;
        }

Answer 1

Maybe I am missing something, but to me this code looks overly cumbersome. Please bear with me, if I am on the wrong track here and misunderstood you.

Regarding the objects that should be deleted, I suggest that you store these in a separate collection that only holds the deleted items. You can the delete them from the ObjectContext.

Instead of calling ApplyCurrentValues I would, I would simply call myObjectContext.SaveChanges(). ApplyCurrentValues has, in this case, the downside that it does not take care of any other entity that has relations to the one you are saving.

MSDN documentation:

Copies the scalar values from the supplied object into the object in the ObjectContext that has the same key.

As the other Segments are already attached to your Treatment, by using SaveChanges(), they will be added to the context automatically or updated if they were already added.

This should make all that manual handling of the EntityStates unnecessary.

EDIT: Now I see where this is going...

Somewhere in you you code - outside of this Save() method - you are deleting the Segment instances. The trouble lies in the problem that your ObjectContext is totally unaware of this. And how should it be...?

You may have destroyed the instance of a certain Segment entity, but as the entities are detached, this means that they have no connection to the ObjectContext. Therefore the context has absolutely no idea of what you have done.

As a consequence, when you attach the treament to it, the context still believes that all Segments are alive because it does not know about the deletion and adds them again to the Treatment like nothing ever happened.

Solution: Like I already said above, you need to keep track of your deleted entities.

In those spots, where you delete the Segments, do not actually delete them, but:

1. Remove() it from the Treatment instance.
2. Move the "deleted" Segment into a collection, e.g. List<Segment>. Let's call it deletedSegments.
3. Pass the deletedSegments collection into the Save() method
4. Loop through this collection and ObjectContect.Delete() them.
5. Do the rest of the remaining save logic as necessary.

Also, like Tomas Voracek mentioned, it is preferable to use contexts more locally. Create it only within the save method instead of passing it as an argument.

Answer 2

Ok try again.

When you say "remove" do you mean mark as deleted.

You are calling ChangeObjectState to change the state to modified.

So if you send 3 down, one deleted, one modified and one unchanged; then all will get marked as modified before save changes is called.