0

I'm struggling to figure out an issue with the Memento pattern. Although I understand it and I'm able to implement it, I must be missing something because it seems to me that if fails when applied to an object that has List type properties.

Consider the following classes:

public class LEDTV
{
    public string Size { get; set; }
    public List<Input> Inputs { get; set; }
    public Manufacturer Manufacturer { get; set; }

    public LEDTV(string size, List<Input> inputs, Manufacturer manufacturer)
    {
        Size = size;
        Inputs = inputs;
        Manufacturer = manufacturer;
    }

    public void AddInput(Input input)
    {
        Inputs.Add(input);
    }

    public string GetDetails()
    {
        string inputs = string.Join(";", Inputs);

        return "LEDTV [Size=" + Size + ", Inputs=[" + inputs + "], Manufacturer=" + Manufacturer + "]";
    }

    public Memento SaveState()
    {
        return new Memento(this);
    }

    public void RestoreState(Memento memento)
    {
        Size = memento.Size;
        Inputs = memento.Inputs;
        Manufacturer = memento.Manufacturer;
    }
}

public class Memento
{
    public string Size { get; set; }
    public List<Input> Inputs { get; set; }
    public Manufacturer Manufacturer { get; set; }

    public Memento(LEDTV ledTV)
    {
        Size = ledTV.Size;
        Inputs = ledTV.Inputs;
        Manufacturer = ledTV.Manufacturer;
    }

    public string GetDetails()
    {
        return "Memento [Size=" + Size + ", Inputs=[" + Inputs + "], Manufacturer=" + Manufacturer + "]";
    }
}

Now consider the following piece of code:

LEDTV ledTV;
Stack<Memento> mementos = new Stack<Memento>();


// Led TV #1
ledTV = new LEDTV("42 inch", new List<Input>() { new Input("VGA", "") }, new Manufacturer("Samsung"));
mementos.Push(new Memento(ledTV));
Console.WriteLine("\nCurrent LedTV : " + ledTV.GetDetails());


// Make changes to Led TV #1
ledTV.Size = "36 inch";
ledTV.Manufacturer = new Manufacturer("Sony");
ledTV.AddInput(new Input("Digital Audio", ""));


// Led TV #2
ledTV = new LEDTV("46 inch", new List<Input>() { new Input("SCART", "") }, new Manufacturer("LG"));
mementos.Push(new Memento(ledTV));
Console.WriteLine("\nCurrent LedTV : " + ledTV.GetDetails());


// Ledt TV #3
ledTV = new LEDTV("50 inch", new List<Input>() { new Input("HDMI", "") }, new Manufacturer("Toshiba"));
Console.WriteLine("\nCurrent LedTV : " + ledTV.GetDetails());


while (mementos.Any())
{
    Console.WriteLine("\nRestoring to previous LED TV");

    ledTV.RestoreState(mementos.Pop());

    Console.WriteLine("\nCurrent LedTV : " + ledTV.GetDetails());
}

So, in a nutshell, I have a LEDTV type that has a ValueType for the Size, a List of available Inputs and a non ValueType for the Manufacturer. The Memento object accepts a LEDTV to save its state. The code snippet sets a LEDTV, saves its state, then does some changes to it, sets it again and saves its state, sets it and again, and finally rollback all saved states, producing the following output:

Current LedTV : LEDTV [Size=42 inch, Inputs=[VGA], Manufacturer=Samsung]

Current LedTV : LEDTV [Size=46 inch, Inputs=[SCART], Manufacturer=LG]

Current LedTV : LEDTV [Size=50 inch, Inputs=[HDMI], Manufacturer=Toshiba]

Restoring to previous LED TV

Current LedTV : LEDTV [Size=46 inch, Inputs=[SCART], Manufacturer=LG]

Restoring to previous LED TV

Current LedTV : LEDTV [Size=42 inch, Inputs=[VGA;Digital Audio], Manufacturer=Samsung]

Everything works as expected, except for the List property, and I can obviously understand why: the list is not a ValueType, so it's passed along by reference. Any change made to it is reflected both in the LEDTV and Memento, because it's the same list in both objects. Surely I can copy the list content into another list, thus creating a new list object but keeping all the references of the object composing the list, but that seems so hacky. Besides, it doesn't seem doable to me to basically have a duplicate class of all the classes that I want to track states, because that's basically what a Memento is. A desirable solution would be a generic Memento implementation that lists the properties of a type and saves each one, but then I would have to overcome the List issue generically, which doesn't seem possible at first glance.

Any help would be appreciated, either in the form of a concrete solution or just pointing out the way, or simply telling me I misunderstood the entire concept.

cneves
  • 43
  • 6
  • The analysis in your next-to-last paragraph sounds totally right to me, but I don't agree that it's hacky. I don't see how you can get around needing a copy of the list. Can be as simple as calling `Inputs.ToList()`. – sblom Oct 07 '20 at 23:50
  • Also, the generic solution here is implemented in many places in and around the framework, usually under the name `Serialization`/`Deserialization`. – sblom Oct 07 '20 at 23:52
  • Then you get into the problem of `List>`. If you want a generic solution, you'll need to use refection, and look for properties that implement `IList` (and perhaps other collection interfaces). If done right,, it would likely still be cheaper than something like serialize/deserialize. – Flydog57 Oct 08 '20 at 02:32
  • Write your memento class so that it's based on read-only and/or immutable types. If you can do that then the rest of the design will be simple. – Enigmativity Oct 08 '20 at 03:48
  • @sblom, Serialization/Deserialization is a bad solution, as the Deserialization would produce a new reference, so the Manufacturer would not reference the original Manufacturer. – cneves Oct 08 '20 at 22:19

1 Answers1

0

One of the advantages of the Memento pattern is that the originator object encapsulation is preserved. But your object is not well encapsulated to begin with since it's exposing it's own internal state in the form of the List of inputs.

I think the real value of this pattern shines in situations where the objects state can be represented by a subset of the exposed data or where the creating the object is expensive.

For example if your LEDTV where sourced out of a database the Memento might only consist of a record ID which could be used to restore the data from the database. And on the other end of the spectrum the LEDT might require some set of expensive queries to a database to create and you can then avoid re-creating it by storing the data in the Memento.

Memento certainly can be done generically but I'm not sure that's always ideal. In the few instances I've seen it used it was tailored to a specific class because there was a single higher level class that needed to be manipulated rather than the small classes aggregated into that higher level class.

So used appropriately it's a cool pattern, and you may have found a use for it. But I would make sure you're design as a whole is SOLID and then think about where to use the pattern in your design.

For something simple just implementing clonable objects might suffice...

MikeJ
  • 1,299
  • 7
  • 10
  • Hi @MikeJ, thank you for your answer. I left the originator out for sake of simplicity and because its usage wouldn't interfere with the problem itself. I understand your point of view regarding objects where the state can be represented by a subset of the exposed data or the database scenario. I was trying to use it for two scenarios: in a MVVM WPF app, where I would create a memento of an object so that if the user presses a Cancel button, I can revert the state of my data object; the other was a undo/redo scenario. I still haven't found a pattern or strategy to address these issues. – cneves Oct 08 '20 at 21:51