Command pattern for simple text editor

Question

I was asked to code a TextPad with following functionality:

display() – to display the entire content
display(n, m) – to display from line n to m
insert(n, text) – to insert text at line n
delete(n) – delete line n
delete(n, m) – delete from line n to m
copy(n, m) – copy contents from line n to m to clipboard
paste(n) – paste contents from clipboard to line n
undo() – undo last command
redo() – redo last command

They expected the textpad to be in memory (not as file). They also expected to handle error gracefully and the program to be menu driven.

Can someone please review the class structure implemented. Can it be improved so that code can be easily modified and extended?

public interface Command {
 void execute();
 void undo();
 }

---

package com.onedirect.oodesign.textpad;
public class PasteCommand implements Command {
 private TextPad textPad;
 private Integer line;
 public PasteCommand(TextPad textPad, Integer line){
 this.textPad = textPad;
 this.line = line;
 }
 @Override
 public void execute() {
 this.textPad.getLines().addAll(line, this.textPad.getClipBoard());
 }
 @Override
 public void undo() {
 for (int i = line; i < this.textPad.getClipBoard().size(); i++) {
 this.textPad.getLines().remove(i);
 }
 }
}

---

package com.onedirect.oodesign.textpad;
public class CopyCommand implements Command {
 private TextPad textPad;
 private Integer from;
 private Integer to;
 public CopyCommand(TextPad textPad, Integer from, Integer to) {
 this.textPad = textPad;
 this.from = from;
 this.to = to;
 }
 @Override
 public void execute() {
 this.textPad.setClipBoard(this.textPad.getLines().subList(from, to));
 }
 @Override
 public void undo() {
 }
}

---

package com.onedirect.oodesign.textpad;
import java.util.List;
public class DeleteCommand implements Command {
 private TextPad textPad;
 private Integer from;
 private Integer to;
 private List<String> lines;
 public DeleteCommand(TextPad textPad, Integer from, Integer to) {
 this.textPad = textPad;
 this.from = from;
 this.to =to;
 }
 @Override
 public void execute() {
 List<String> content = textPad.getLines();
 this.lines = content.subList(from, to);
 for (int i = from; i < to && i<content.size(); i++) {
 content.remove(i);
 }
 }
 @Override
 public void undo() {
 List<String> content = textPad.getLines();
 content.addAll(from, this.lines);
 }
}

---

package com.onedirect.oodesign.textpad;
import java.util.List;
public class DisplayCommand implements Command {
 private TextPad textPad;
 private int m;
 private int n;
 public DisplayCommand(TextPad textPad, int m, int n) {
 this.textPad = textPad;
 this.m = m;
 this.n = n;
 }
 public DisplayCommand(TextPad textPad) {
 this.textPad = textPad;
 this.m = 0;
 this.n = this.textPad.getLines().size();
 }
 @Override
 public void execute() {
 List<String> list = this.textPad.getLines();
 list.subList(m, n).forEach(System.out::println);
 }
 @Override
 public void undo() {
 }
}

---

package com.onedirect.oodesign.textpad;
public class InsertCommand implements Command{
 private TextPad textPad;
 private Integer line;
 private String text;
 public InsertCommand(TextPad textPad, Integer line, String text) {
 this.line = line;
 this.textPad = textPad;
 this.text = text;
 }
 @Override
 public void execute() {
 this.textPad.getLines().add(line, text);
 }
 @Override
 public void undo() {
 this.textPad.getLines().remove((int)line);
 }
}

---

package com.onedirect.oodesign.textpad;
import java.util.ArrayList;
import java.util.List;
import java.util.Stack;
public class TextPad {
 private List<String> lines;
 private Stack<Command> undo;
 private Stack<Command> redo;
 private List<String> clipBoard;
 public TextPad() {
 this.lines = new ArrayList<>();
 this.undo = new Stack<>();
 this.redo = new Stack<>();
 this.clipBoard = new ArrayList<>();
 }
 public List<String> getClipBoard() {
 return this.clipBoard;
 }
 public void setClipBoard(List<String> clipBoard) {
 this.clipBoard = clipBoard;
 }
 public List<String> getLines() {
 return lines;
 }
 public void display() {
 DisplayCommand displayCommand = new DisplayCommand(this);
 displayCommand.execute();
 }
 public void display(int m, int n) {
 DisplayCommand displayCommand = new DisplayCommand(this, m, n);
 displayCommand.execute();
 }
 public void insert(int n, String text) {
 clearRedo();
 InsertCommand insertCommand = new InsertCommand(this, n, text);
 insertCommand.execute();
 undo.push(insertCommand);
 }
 public void delete(int n) {
 delete(n, n);
 }
 public void delete(int n, int m) {
 clearRedo();
 DeleteCommand deleteCommand = new DeleteCommand(this, n, m);
 deleteCommand.execute();
 undo.push(deleteCommand);
 }
 public void copy(int n, int m) {
 clearRedo();
 CopyCommand copyCommand = new CopyCommand(this, n, m);
 copyCommand.execute();
 }
 public void paste(int n) {
 clearRedo();
 PasteCommand pasteCommand = new PasteCommand(this, n);
 pasteCommand.execute();
 undo.push(pasteCommand);
 }
 public void undo(){
 if(!undo.isEmpty()) {
 Command command = undo.pop();
 command.undo();
 redo.push(command);
 }
 }
 public void redo(){
 if(!redo.isEmpty()) {
 Command command = redo.pop();
 command.execute();
 undo.push(command);
 }
 }
 private void clearRedo() {
 while (!redo.isEmpty()) {
 redo.pop();
 }
 }
}

---

package com.onedirect.oodesign.textpad;
import java.util.Scanner;
public class TextPadDriver {
 public static void main(String[] args) {
 TextPad textPad = new TextPad();
 Scanner in = new Scanner(System.in);
 while (true) {
 System.out.println("Please enter I to insert,"
 + " D to delete, C to copy , P to paste, "
 + "U to undo, R to redo, S to show, another to end");
 String c = in.next();
 switch (c) {
 case "I" :
 System.out.println("Give Line Number");
 Integer n = in.nextInt();
 System.out.println("Give Text");
 String text = in.next();
 textPad.insert(n, text);
 break;
 case "D":
 System.out.println("Give Line Number");
 Integer from = in.nextInt();
 System.out.println("Give Line Number");
 Integer to = in.nextInt();
 textPad.delete(from, to);
 break;
 case "C":
 System.out.println("Give Line Number");
 Integer fromCopy = in.nextInt();
 System.out.println("Give Line Number");
 Integer toCopy = in.nextInt();
 textPad.copy(fromCopy, toCopy);
 break;
 case "P":
 System.out.println("Give Line Number");
 Integer line = in.nextInt();
 textPad.paste(line);
 break;
 case "U":
 textPad.undo();
 break;
 case "R":
 textPad.redo();
 break;
 case "S":
 textPad.display();
 break;
 default:
 return;
 }
 }
 }
}

enter image description here

• Graipher: Since this has popped up in the close vote queue, does this code currently work as expected?
• Roman: @Manu, is it possible for you to provide us the TextPadDriver?
• Manu: @Roman I have added textPadDriver. But it doesn't cover all use cases.
• Manu: @Graipher It is working and might not cover all edge cases. But I am more concerened about the structure than correctness of the code.
• Roman: @Manu, thank you. I will edit my answer soon based on the TextPadDriver

Answer 1

Command Pattern

[The] Command [Pattern] decouples the object that invokes the operation from the one that knows how to perform it.

In our case this means, that a Command invokes a method of TextPad. Though it is the way around and TextPad invokes a Command.

For example, when we look into the method display in TextPad:

public void display() {
 DisplayCommand displayCommand = new DisplayCommand(this);
 displayCommand.execute();
}

Actually the logic should not be in a Command instead it needs to be in the TextPad itself. In the following I extract the logic out of the DisplayCommand into TextPad

public void display() {
 List<String> list = this.textPad.getLines();
 list.subList(m, n).forEach(System.out::println);
}

After that we can invoke the method in DisplayCommand.

@Override
public void execute() {
 textPad.display();
}

---

The TextPad should work totally independent of Command. Additionally the fields Stack<Command> undo and Stack<Command> redo do not belong into TextPad but inside TextPadDriver.

---

Why this Way?

Currently the code base violates against some opp-principles.

Feature Envy

A method accesses the data of another object more than its own data.

All implementations of Command operates on TextPad. For that the TextPad needs to offer its fields like lines and clipBoard. But actual the a Command should Tell, Don't Ask a TextPad.

So the preferred way is

@Override
public void execute() {
 textPad.display();
}

Law of Demeter

Only talk to your immediate friends.

This "problem" coheres with the Feature Envy, because every Command gets an instance of TextPad but only needs a sub information of it like lines or clipBoard.

this.textPad.getLines().remove(i)

The Law of Demeter do not allow the above statements, because you talk via remove to a "frind" (lines) of your "frind" (textPad), but you are only allow to talk to your "immediat friend" (textPad).

A valid statement under the Law of Demeter is

this.textPad.removeLine(i)

---

Empty Methods

@Override
public void undo() {
}

To have a empty method is totally valid, but it is better to leave it with a comment

@Override
public void undo() {
 // nothing to do here
}

The benefits are that as a reader, I know that you are aware that you are not implementing it, and that you will know in the future that you have not forgotten the implementation.

---

Constructor Chaining

public DisplayCommand(TextPad textPad, int m, int n) {
 this.textPad = textPad;
 this.m = m;
 this.n = n;
}
public DisplayCommand(TextPad textPad) {
 this.textPad = textPad;
 this.m = 0;
 this.n = this.textPad.getLines().size();
}

It exists a constructor for all arguments DisplayCommand(TextPad textPad, int m, int n) and a constructor, which takes only one argument DisplayCommand(TextPad textPad) and sets m and n to some default values.

It is possible to chain the one-argument constructor to the full-args constructor.

public DisplayCommand(TextPad textPad, int m, int n) {
 this.textPad = textPad;
 this.m = m;
 this.n = n;
}
public DisplayCommand(TextPad textPad) {
 this(textPad, 0, textPad.getLines().size());
}

• Manu: I have added TextPad Driver Class. public void display() { DisplayCommand displayCommand = new DisplayCommand(this); displayCommand.execute(); } public void display() { List<String> list = this.textPad.getLines(); list.subList(m, n).forEach(System.out::println); } To which classes the above two methods should belong to?