dcastro / MementoContainer

5 stars 2 forks source link

State of child collection in class not being preserved when rolled back #3

Closed Guerillah closed 6 years ago

Guerillah commented 6 years ago

Thanks for this library... I like this implementation so far the best of all of the others available for C#.

In experimenting with this library I am unsure of how to get the framework to keep the state of a collection inside a class as it is currently not working. I have the following fake Person class.

[MementoClass] public class Person { public string Name { get; set; } public List<string> Addresses { get; set; } public DateTime DateOfBirth { get; set; } }

When I run the following code, it will not restore the addresses that existed before I cleared them. What must I do to make that happen?

`var person = new Person { Addresses = new List { "12180 N Lazy River", "4311 W Summit Ranch" }, DateOfBirth = DateTime.Now, Name = "David D" }; var personList = new List { person }; var memento = Memento.Create().RegisterProperty(person, p=>p.Name).RegisterProperty(person, p=>p.Addresses).RegisterCollection(personList);

        try
        {
            person.Name = "Changed";
            person.Addresses.Clear();
            throw new Exception();
        }
        catch (Exception x)
        {
            memento.Rollback();
        }

        var addressCount = person.Addresses.Count; //Equals 0 when should equal 2 if restored correctly.`
dcastro commented 6 years ago

Hi @Guerillah,

Thanks, I'm glad to hear this library is being useful for you.

It's been about 5 years since I wrote this, so I don't exactly remember the details, but have you tried adding the [MementoCollection] attribute to the Addresses property?

Guerillah commented 6 years ago

Yea, I saw that it was written a while ago. Adding the [MementoCollection] attribute to Addresses does not resolve the issue. If I take actions on an individual Person the Addresses collection is preserved. It is only when I have a List of X that this issue arises. For example, changing the code to the following will result in the addressCount being 2.

` var person = new Person { Addresses = new List { "12180 N Lazy River", "4311 W Summit Ranch" }, DateOfBirth = DateTime.Now, Age = 34, Name = "David D" }; var personList = new List { person }; var memento = Memento.Create().Register(person);

        try
        {
            person.Name = "Changed";
            person.Addresses.Clear();
            throw new Exception();
        }
        catch (Exception x)
        {
            memento.Rollback();
        }

        var addressCount = person.Addresses.Count;`

I'll work with the source code and see if I can figure it out.

Guerillah commented 6 years ago

@dcastro

Easy fix...

Change the CreateCollectionMemento method call inside the RegisterCollection method to cascade. The more permanent fix would be to make a boolean cascade argument for the RegisterCollection method so that it can be configured as the user see's fit.


        /// Registers a collection.
        /// After <see cref="IMemento.Rollback"/> is called, the collection will contain the same elements as at the time of registration.
        /// </summary>
        /// 
        /// <typeparam name="T">The type of the elements in the collection.</typeparam>
        /// <param name="collection">The collection being registered.</param>
        /// <returns>This IMemento instance.</returns>
        public IMemento RegisterCollection<T>(ICollection<T> collection)
        {
            collection.ThrowIfNull("collection");

          //var memento = Factory.CreateCollectionMemento(collection, false);
            var memento = Factory.CreateCollectionMemento(collection, true);

            Components.Add(memento);
            return this;
        }
dcastro commented 6 years ago

@Guerillah Thanks a lot for digging into this!

Yea I think it would be best to add a parameter to RegisterCollection. If you're willing to submit a PR with this fix, I'll push it up to NuGet! :)