# Sharing my Memento Exercise

**URL:** <https://forum.codewithmosh.com/t/sharing-my-memento-exercise/7351>\
**Category:** Design Patterns\
**Created:** [September 14, 2021, 6:56pm UTC](https://forum.codewithmosh.com/t/sharing-my-memento-exercise/7351 "2021-09-14T18:56:00Z")\
**Posts on this page:** 2\
**Page:** 1

<div class="post-metadata">

**Author:** ![lily](https://avatars.discourse-cdn.com/v4/letter/l/97f17d/32.png) [@lily](https://forum.codewithmosh.com/u/lily)\
**Post date:** [September 14, 2021, 6:56pm UTC](https://forum.codewithmosh.com/t/sharing-my-memento-exercise/7351/1 "2021-09-14T18:56:00Z")

</div>

Hello everyone!  
I have a slightly different implementation than Mosh’s and wondering if anyone can give me feedback on mine? So what I did was I am treating Document to hold current state and only add prevStates to the Caretaker class. With Mosh’s implementation, it feels as though we are saving the current state to the Caretaker (History) class so when we first call restoreState(), it’s actually restoring the current state which confused me…

**Document Class**

```auto
public class Document {
    private String content;
    private String fontName;
    private int fontSize;

    public Document() {
        /**
         * sets the default value when a new document is created to represent the real world
         * document where initially the document is empty
         **/
        this.content = "";
        this.fontName = "Arial";
        this.fontSize = 12;
    }

    // restoring the internal state of Document to the previous state
    public void undo(DocumentState docState) {
        this.content = docState.getContent();
        this.fontName = docState.getFontName();
        this.fontSize = docState.getFontSize();
    }

    public String getContent() {
        return content;
    }

    public DocumentState setContent(String content) {
        DocumentState state = new DocumentState(this);
        this.content = content;
        return state;
    }

    public String getFontName() {
        return fontName;
    }

    public DocumentState setFontName(String fontName) {
        DocumentState state = new DocumentState(this);
        this.fontName = fontName;
        return state;

    }

    public int getFontSize() {
        return fontSize;
    }

    public DocumentState setFontSize(int fontSize) {
        DocumentState state = new DocumentState(this);
        this.fontSize= fontSize;
        return state;
    }

    @Override
    public String toString() {
        return "Document{" +
                "content='" + content + '\'' +
                ", fontName='" + fontName + '\'' +
                ", fontSize=" + fontSize +
                '}';
    }

    public void printCurrentState() {
        System.out.println(this.toString());
    }
}

```

**DocumentState (Memento)**

```auto
public class DocumentState {

    private String content;
    private String fontName;
    private int fontSize;

    public DocumentState(String content, String fontName, int fontSize) {
        this.content = content;
        this.fontName = fontName;
        this.fontSize = fontSize;
    }

    public DocumentState(Document doc) {
        this.content = doc.getContent();
        this.fontName = doc.getFontName();
        this.fontSize = doc.getFontSize();
    }

    public String getContent() {
        return content;
    }

    public String getFontName() {
        return fontName;
    }

    public int getFontSize() {
        return fontSize;
    }

}

```

StateManager (Caretaker)

```auto

public class StateManager {
    private Stack<DocumentState> states = new Stack<>();

    public void push(DocumentState state) {
        states.add(state);
    }

    public DocumentState pop() {
        DocumentState prevState = states.peek();
        states.pop();
        return prevState;
    }
}

```

Driver class:

```auto
package com.codewithmosh.memento;

public class Driver {
    public static void main(String[] args) {
        Document document = new Document();
        StateManager history = new StateManager();

        history.push(document.setContent("hello how are you?"));
        history.push(document.setFontName("calibri"));

        document.printCurrentState();

        document.undo(history.pop());

        document.printCurrentState();

        document.undo(history.pop());

        document.printCurrentState();
    }
}

```

---

<div class="post-metadata">

**Author:** ![jmrunkle](https://sea2.discourse-cdn.com/flex020/user_avatar/forum.codewithmosh.com/jmrunkle/32/3765_2.png) [@jmrunkle](https://forum.codewithmosh.com/u/jmrunkle)\
**Post date:** [August 10, 2022, 11:41pm UTC](https://forum.codewithmosh.com/t/sharing-my-memento-exercise/7351/2 "2022-08-10T23:41:05Z")

</div>

Not sure what sort of feedback you were looking for. The implementation seems fine, but there are some mild things I could nitpick.

You have a circular relationship between Document and DocumentState that seems unnecessary - there is no need for DocumentState to depend on Document, just pass the parameters to its constructor (or if there were many, I would recommend using the Builder pattern to fix that).

Some additional minor details:

- DocumentState’s fields should be final so that it is clear this is an immutable object
- Stack.pop() returns the item that was removed from the stack so you could replace your current implementation with a one liner: `return states.pop();`
- Not that Mosh fixed this, but the current implementation does not handle the empty history case (nor does it provide visibility to check if the stack is empty).

Personally, I would rather have the Document class compose the History class so that it can directly push and pop from history in the appropriate methods. It would also allow us to have an `undo()` method without parameters which is pretty nice to have. It would also mean you do not need to clutter the API with methods that return DocumentState for no reason other than to allow it to get passed to the StateManagement class.

Cheers!