0

Across the internet I come across with examples of implementation of memento pattern which I consider are fully incorrect. They can be written in both Java and C#.

Here a several of them

  1. Incorrect implementation of Memento pattern 1
  2. Incorrect implementation of Memento pattern 2
  3. Incorrect implementation of Memento pattern 3

The code:

public class Originator
{
    private string _state; //the private field of originator that shouldn't be exposed!!!!

    public Memento CreateMemento()
    {
        return new Memento(_state);
    }

    public void SetMemento(Memento memento)
    {
        _state = memento.GetState();
    }
}


public class Memento
{
    private string _state;

    public Memento(string state)
    {
        _state = state;
    }

    public string GetState()
    {
        return _state;  // here caretaker can access to private field of originator!!!
    }
}


public class Caretaker
{
    public Memento Memento { get; set; }
}

In code I left comments that should describe situation.

The caretaker class can read private field of originator through memento that violates one of the main principles of memento pattern:

The object's encapsulation must not be violated.

So the question is am I right?

vborutenko
  • 4,323
  • 5
  • 28
  • 48
  • I am not a c# specialist, but at least in Java `String`s are immutable, so "exposing" the field should not be a concern? – Lino Mar 25 '20 at 11:22
  • @Lino what if it would be for example int type? – vborutenko Mar 25 '20 at 11:24
  • @Lino so are in `c#` – Cleptus Mar 25 '20 at 11:24
  • In java,for example in the first Incorrect implementation of Memento pattern 1 – vborutenko Mar 25 '20 at 11:25
  • 1
    `int` will be copied at `Memento` constructor, so `Memento._state` and `Originator._state` are 2 different, independent `int`s (same as `string`) – vasily.sib Mar 25 '20 at 11:26
  • Yes,technically they are different but anyway we can say for 100% sure that Memento._state ==Originator._state.And so we caretaker can know the value of PRIVATE field of originator – vborutenko Mar 25 '20 at 11:27
  • 2
    @vborutenko, no try `object.ReferenceEquals(Memento._state, Originator._state)`. Also, change to `Originator._state` will not change `Memento._state` and vice versa – vasily.sib Mar 25 '20 at 11:29
  • 1
    Looks like a bit of missundestanding on reference vs. value types – Cleptus Mar 25 '20 at 11:31
  • @vasily.sib but the caretaker class ,in other words, can read the value of private field through momento.It can know the value of the private field.For example the caretakerclass will know that the private field of originator will have for example "someprivatefield" value. Doesn't this violate encapsulation? – vborutenko Mar 25 '20 at 11:34
  • @bradbury9,the missundestanding here that I think that caretaker shouldn't know the value of internal state of originator – vborutenko Mar 25 '20 at 11:38
  • 1
    @vborutenko, no, it have no access to `Originator` private field. It stores the **value** (a copy of that `string`/`int`) that was in that field some time ago. So, if `Originator` somehow change his `_state`, `Memento` will never know about this. Same time, if `Momento` somehow change his `_state` (his own copy of that `string`/`int` value) - this will never do anything to `Originator`s `_state` – vasily.sib Mar 25 '20 at 11:38
  • OK see the example.Caretaker ask originator to make a new memento.var memento = originator.CreateMemento() . Console.Writeline ( "Hi. I'm caretaker and I know the value of private field '_state' of originator. It equals " + memento.GetState).Doesn't this violate encapsulation? – vborutenko Mar 25 '20 at 11:40
  • 3
    @vborutenko, ok, now i see what bothers you. Exposing a **value** (a copy) of a private field is not violates encapsulation. As of your example, there should be _"Hi. I'm caretaker and I know the value of private field '_state' of originator that was equals " + memento.GetState() + " **some time ago, it may be changed now, IDK**."_ – vasily.sib Mar 25 '20 at 11:51

3 Answers3

1

So the question is am I right?

Yes, you are right: the examples are all incorrect implementations of the Memento design pattern, because the Memento class exposes its internal dependency through a public method. As you noted, this allows the Caretaker (or any other class) to gain private information about the Originator.

Previous answers focus on the state of the internal field and whether the Memento state can vary independently of the Originator. But encapsulation can be violated in more ways than by exposing state. It can also be violated by exposing dependencies. The public getter method exposes the dependency that Originator has on String.

I wish I could say that misinformation is unusual regarding design patterns; but you will find inaccurate and misleading articles about design patterns across the Internet, including here on SO and on Wikipedia.

I happen to like https://refactoring.guru/design-patterns/memento but it's never a bad idea to crosscheck against the GoF book.

jaco0646
  • 15,303
  • 7
  • 59
  • 83
0

Encapsulation doesn't mean that you shouldn't be able to access the data at all, you just shouldn't be able to access it directly. You should only be able to access it via public methods :)
This link is probably able to explain it better than i am:

https://www.geeksforgeeks.org/encapsulation-in-java/

So encapsulation is kept in your code example, since the state can only be retrieved via a public get method.

DBJsName
  • 9
  • 1
  • In originator class we 'hide' the value of the state make it private.We don't want that someone know the value of that field.But in the example we see that caretaker can know this value – vborutenko Mar 25 '20 at 11:36
  • If you dont want to let other classes know about a private property, dont add a public getter that retrieves its value. – Cleptus Mar 25 '20 at 11:37
  • It is implementation of memento pattern from the internet.I provided several links. – vborutenko Mar 25 '20 at 11:39
  • 2
    But you made a false assumption because you failed to notice the difference between value and reference types – Cleptus Mar 25 '20 at 11:41
  • No,please see the last comments to the questions – vborutenko Mar 25 '20 at 12:06
  • vborutenko - If you look at the link i referenced. In the very first example of what encapsulation is, they make the private variables accessible via a public get method. You claim the reason why you think the implementations are wrong is because it doesn't adhere to encapsulation, because of the public get method in Memento, but the public get method is a part of encapsulation.Here's another link where this is used, if you are still in doubt : https://www.tutlane.com/tutorial/csharp/csharp-encapsulation – DBJsName Mar 25 '20 at 12:17
0

If we add some logic to check how the variables evolve, you will notice how Originator and Memento are not sharing the same data.

public class Originator
{
    private string _state = "originator_initial_state";

    public Memento CreateMemento()
    {
        return new Memento(_state);
    }

    public void DoSomeLogic()
    {
        _state = _state + " modified";
    }

    public void SetMemento(Memento memento)
    {
        _state = memento.GetState();
    }
}

public class Memento
{
    private string _state;

    public Memento(string state)
    {
        _state = state;
    }

    public string GetState()
    {
        return _state;
    }
}

Now lets work with them:

Originator myOriginator = new Originator();
Memento memento1 = myOriginator.CreateMemento();

myOriginator.DoSomeLogic();

Memento memento2 = myOriginator.CreateMemento();

string memento1_state = memento1.GetState();
string memento2_state = memento2.GetState();

Console.WriteLine(memento1_state);
Console.WriteLine(memento2_state);

You will notice that memento1_state and memento2_state are different, because they do not share the same underlying object.

Cleptus
  • 3,446
  • 4
  • 28
  • 34