1
\$\begingroup\$

I been trying to implement a text processor in JavaScript that can handle 4 different types of operations: append, backspace, undo, redo.

The input for this text processor is an array of arraries of a single element or tuples. For example:

  const input = [['APPEND', 'Hey'], ['APPEND', ' there'], ['APPEND', '!']]
  textProcessor.process(input)
  textProcessor.text // Hey there!
  const input = [['APPEND', 'Hey'], ['APPEND', ' there'], ['APPEND', '!'], ['UNDO'], ['UNDO']]
  textProcessor.process(input)
  textProcessor.text // Hey
  const input = [['APPEND', 'Hey'], ['APPEND', ' there'], ['APPEND', '!'], ['UNDO'], ['UNDO'], ['REDO'], ['REDO']]
  textProcessor.process(input)
  textProcessor.text // Hey there!
  const input = [['APPEND', 'Hey'], ['APPEND', ' there'], ['APPEND', '!'], ['BACKSPACE']]
  textProcessor.process(input)
  textProcessor.text // Hey there

Here is my implementation

class TextProcessor {
    constructor() {
      this.undos = []
      this.redos = []
      this.text = ''
      this.operations = {
        APPEND: (text) => {
          this.undos.push(this.text)
          this.redos.length = 0
          return this.text + text
        },
        BACKSPACE: () => {
          this.undos.push(this.text)
          this.redos.length = 0
          return this.text.slice(0, -1)
        },
        UNDO: () => {
          const undo = this.undos.pop() ?? ''
          this.redos.push(this.text)
          return undo
        },
        REDO: () => {
          this.undos.push(this.text)
          return this.redos.pop() ?? this.text
        },
      }
    }

    process(input) {
      input.forEach(([operation, text]) => {
        this.text = this.operations[operation](text) ?? this.text
      })
    }
  }
  

I used an object to map the various operation names to their corresponding functions. Not sure if this is better than switch-case statements.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Unusual undo redo

Your undo and redo operations have a strange behavior in that they are them selves undo-able. Generally undo and redo operations are themselves not undo-able.

Bug?

Unknown commands will throw the error TypeError: this.operations[operation] is not a function

SEMICOLONS!

I think I mentioned semicolons last time I reviewed your code. If you do not wish to use them you should be familiar with ASI

Identifiers

Try to avoid string identifiers as they are memory hungry ad can be much slower depending on the number and similarity of identifiers.

One method of creating unique identifiers is via an enumeration object.

Example

const operators = {
    append: 0,
    undo: 1,
    redo: 2,
    backspace: 3,
}

The rewrite uses a function Enum that returns an object containing named integer Identifiers.

Encapsulation

Protect the state of the object. All properties of an object exposed to other code can not be trusted. Without a trusted state you can not guarantee that your code will run as expected.

A well designed encapsulated object (once tested) can NOT fail.

JavaScript has an excellent OO encapsulation model.

  • Closure is used to encapsulate state within the Object that do not require additional language tokens to access (eg this)
  • Object functions
    • Object.freeze will set all properties to writable: false, and the object to immutable.
    • Object.seal properties remain writable but the object is set to immutable.

Using closure to hold the objects state, you then create an interface to provide an interface to the state. The interface is just an object (frozen or sealed) that uses getters, setters, and functions to access state and control behavior

Be efficient

Always write code to be as efficient as possible.

Yes this is a balance between productivity and code efficiency but with a constant eye on efficiency you become more adept at writing efficient code

Inefficient undo buffer

Your undo is expensive as it stores the complete text for every action that can be undone. You need only store each operation and associated data in the undo buffer.

This will make your code more complex but for large documents it will run much quicker and use far less memory. Clients don't see or care how things are done, they only care about what it does, if that include slow response and power hungry it is never a plus.

Rewrite

The rewrite adds a little functionality to help demonstrate some of the points above regarding the interface getters and setters.

Usage remains the same

Behavior is a little different

  • Undo and redo are not undo-able commands

  • It uses an named integer values to identify operations. Which is a static property of TextProcessor.operators and internally accessed via operators

  • The function TextProcessor.process ensures input is an array, filters out non arrays to avoid errors, and checks for valid operation before attempting to call the operation, again to avoid throwing errors.

  • For every undo-able operation there is an equivalent redo operation. This means that only the operation and operation data need be stored in the undo buffer.

  • All objects accessible by unrelated code are frozen to ensure that state can be trusted and maintained.

  • Individual operations are not available via the interface but would be easy to add if needed.

const Enum = (baseId, ...names) => Object.freeze(
    names.reduce((obj, name, i) => (obj[name] = i + baseId | 0, obj), {})
);  
const TextProcessor = (() => {
    function TextProcessor() {
        const undos = [];
        const undoOperations = {
            [operators.APPEND](txt) { text = text.slice(0, -txt.length) },
            [operators.BACKSPACE](txt) { text += txt },                      
            [operators.CLEAR](txt) { text = txt },                      
        };
        const  undoable = (operation, ...data) => {
            undos.length = undoPos++;
            undos.push({operation, data});
        }
        const redoUndoable = () => undoPos++;
        const operations = {
            [operators.APPEND](txt) {
                txt = "" + txt;      // forces txt to be a string
                addUndoable(operators.APPEND, txt);
                text += txt;
            },
            [operators.BACKSPACE]() {
                addUndoable(operators.BACKSPACE, text[text.length - 1]);
                text = text.slice(0, -1);      
            },    
            [operators.CLEAR]() {
                addUndoable(operators.CLEAR, text);
                text = "";       
            },              
            [operators.UNDO]() {
                if (undoPos) {
                    const op = undos[--undoPos];
                    undoOperations[op.operation](...op.data);
                }            
            },        
            [operators.REDO]() {
                if (undoPos < undos.length) {
                    const op = undos[undoPos];
                    addUndoable = redoUndoable;
                    operations[op.operation](...op.data);
                    addUndoable = undoable;
                }                
            }
        };  
        var undoPos = 0, text = "", addUndoable = undoable;        
        return Object.freeze({
            get text() { return text },
            set text(txt) {
                operations[operators.CLEAR]();
                operations[operators.APPEND](txt);
            },
            process(input) {
                if (Array.isArray(input)) {
                    input = input.filter(operation => Array.isArray(operation));
                    for (const [opName, data] of input) { 
                        operations[operators[opName]]?.(data);
                    }
                }
            } 
        });
    }
    const operators = Enum(1, "APPEND", "BACKSPACE", "UNDO", "REDO", "CLEAR");
    return Object.freeze(Object.assign(
        TextProcessor,
        operators
    ));
})();

const textProcessor = TextProcessor()
textProcessor.process([
  ['APPEND', 'Hey'], 
  ['APPEND', ' there'], 
  ['APPEND', '!'], 
  ['BACKSPACE'], ['BACKSPACE'], ['BACKSPACE'], ['BACKSPACE'],
  ['APPEND', '$'],
  ['UNDO'], ['REDO'],
  ['UNDO'], ['UNDO'], ['UNDO'], ['UNDO'], ['UNDO'], ['UNDO']
  ['REDO'], ['REDO'], ['REDO'], ['REDO'], ['REDO'], ['REDO'],
  ['UNDO'],
  ['APPEND', 'e'],
  ['APPEND', 'r'],
  ['APPEND', 'e'],
  ['APPEND', '$'], ['UNDO'],
  ['APPEND', '!'],
  ['CLEAR'],
  ['UNDO'],
]);

console.log("'" + textProcessor.text + "'" + " === 'Hey there!' " + (textProcessor.text === "Hey there!"));

\$\endgroup\$
3
  • \$\begingroup\$ Great answer. I noticed you used var instead of let , is it possible use let or in this case better to use var for some specific reason I'm missing? \$\endgroup\$ Apr 3, 2021 at 8:27
  • \$\begingroup\$ @dariosicily There is no reason you can not use a block scoped variable in function scope. i am a strong believer that all code should show clear intent and understanding. \$\endgroup\$
    – Blindman67
    Apr 3, 2021 at 12:02
  • \$\begingroup\$ Thank you for your time and your explanation. \$\endgroup\$ Apr 3, 2021 at 12:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.