-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Memento pattern #40
base: master
Are you sure you want to change the base?
Conversation
|
||
val memento = originator.saveToMemento() | ||
|
||
assert.that(memento.state, `is`(memento.state)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case this method will always be true, isn't it. Because here you are comparing memento state with the same memento state, not with the originator. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well done @ConradoMateu 👍
mementoList.add(memento) | ||
} | ||
|
||
fun getMemento(index: Int) : Memento<T>? = mementoList[index] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really the best way to do this in Kotlin? I see little sense in wrapping a mutable list 🤔
Close #20
I think this pattern has no sense in Kotlin. We could join
Originator
andMemento
and we could makeCareTaker
dissappear because it is only a wrapper of a List of Mementos. Memento has a immutable state and if you want to modify it's state you would create a new Memento with another state and store them in a List in case you want to get back to some of them.What do you think?