1
\$\begingroup\$

I have built a simple finite state machine that models an even simpler fictitious bank account system.

Any given account can be in one of three states at any time: Open, Closed, or Held.

I have a few different methods that transition from one state to another:

  • reopen: closed -> open
  • close : open / held -> closed
  • place_hold : open -> held
  • remove_hold : held -> open

I have used an ES6 class and prototypes to implements this. three different objects, each with its own methods, are set to be the __proto__ of any given BankAccount object.

The problem with this is that changing the __proto__ of an object removes its ability to call its class's methods, which is a problem.

Here is a stripped down version of my code:

const open ={stateName:"Open",
  close: function(){
    this.changeState(closed);
  },
  reopen: function(){
    return;
  },
  place_hold: function(){
    this.changeState(held);
  },
  remove_hold: function(){
    return;
  }
}

const held ={stateName:"Held",
  close: function(){
    this.changeState(closed);
  },
  reopen: function(){
    return;
  },
  place_hold: function(){
    return;
  },
  remove_hold: function(){
    this.changeState(open);
  }
}

const closed ={stateName:"Closed",
  close: function(){
    return;
  },
  reopen: function(){
    this.changeState(open);
  },
  place_hold: function(){
    return;
  },
  remove_hold: function(){
    return;
  }
}

class BankAccount {
  constructor() {
    this.balance = 0;
    this.__proto__ = open;
    this.changeState = state=>this.__proto__ = state;
  }
  otherStuff(){
    // this cannot be called...
  }
}

What would be a more proper way to do this, keeping our accounts able to access the class's methods, as well as the syntax used to access the methods implemented through the state machine?

\$\endgroup\$

closed as off-topic by Dannnno, Sᴀᴍ Onᴇᴌᴀ, Ben Steffan, Mast, 200_success Mar 9 '18 at 21:00

This question appears to be off-topic. The users who voted to close gave these specific reasons:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – Sᴀᴍ Onᴇᴌᴀ, Ben Steffan, 200_success
  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Dannnno, Mast
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ Welcome to Code Review! I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. The example code that you have posted is not reviewable in this form because it leaves us guessing at your intentions. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR? \$\endgroup\$ – Dannnno Feb 28 '18 at 13:29
  • \$\begingroup\$ @Dannnno the code i posted is working code. What information is lacking to remove any ambiguity? Please, help me make my question on topic \$\endgroup\$ – Sebastián Mestre Feb 28 '18 at 18:38
  • \$\begingroup\$ It's working code that doesn't appear to serve any actual purpose, i.e. example code. If there is a purpose I missed, then ignore this. Otherwise, an example of how you would use this would be greatly appreciated. \$\endgroup\$ – Dannnno Feb 28 '18 at 18:46
  • \$\begingroup\$ @Dannnno it belongs to a project ( the project is a "favor bank" to keep track of "debt" between me and some friends, back end will be done in node.js ) that is very early in development. This has not been integrated into the project yet. Asking me to put example code of this in use would be asking me to produce purposeless, fictitious code that would otherwise not exist. \$\endgroup\$ – Sebastián Mestre Feb 28 '18 at 18:54
1
\$\begingroup\$

Object properties.

If you overwrite the prototype you will lose existing properties.

There are other solutions using the class syntax, but it is important to note the following.

JavaScript classes, introduced in ECMAScript 2015, are primarily syntactical sugar over JavaScript's existing prototype-based inheritance. The class syntax does not introduce a new object-oriented inheritance model to JavaScript.

from MDN Classes

Using class has some major problems, and in my opinion should be avoided as the same and better can be done using standard syntax for objects.

A word on code.

  • You don't need to add return to a function. Its automatic.
  • Use function shorthand when adding functions to objects.
  • Don't mix naming styles. You have snake_case and camelCase.

Apply that to

const held ={stateName:"Held",
  close: function(){
    this.changeState(closed);
  },
  reopen: function(){
    return;
  },
  place_hold: function(){
    return;
  },
  remove_hold: function(){
    this.changeState(open);
  }
}

Becomes

const held = {
  stateName: "Held",
  close() {              // function shorthand
    this.changeState(closed);
  },
  reopen() {},           // return implied Same as return undefined;
  placeHold() {},        // camelCase
  removeHold() {
    this.changeState(open);
  }
};  // objects have semicolons..

Object properties.

When you are creating objects with dynamic properties you can assign properties directly to the object using Object.assign(obj, properties) This has some performance benefits as well.

The following is an example of an instanceable account state machine using object assign to assign the the state.

const Account = (() => {
    const actions = {
        close() { this.transition(states.closed) },
        hold()  { this.transition(states.held) },
        open()  { this.transition(states.open) },
    };
    const nulls = {close(){ }, hold(){ }, open(){ }};
    const states = {
        open : {
            state: "Open",
            close: actions.close,
            hold: actions.hold,
        },
        held : {
            state: "Held",
            close: actions.close,
            open: actions.open,
        },
        closed : {
            state: "Closed",
            open: actions.open,
        }
    };
    return function(){
        const account = {
            ...states.closed,  // default State
            balance : 0,
            transition(state) { Object.assign(account, nulls, state) },
            statement() { console.log(account) },
        };            
        return account;
    };
})();

To use

const foo = Account(); 
// or
const foo = new Account();

foo.close();
foo.open();
foo.hold();
foo.statement();
\$\endgroup\$
  • \$\begingroup\$ How does this do with regard to memory usage? Do the actions get passed by reference or are they being copied? \$\endgroup\$ – Sebastián Mestre Feb 28 '18 at 22:51
  • 1
    \$\begingroup\$ @SebastiánMestre Object.assign is a shallow copy so state is by reference, (except the state string containing the state name, like all strings it is copied replacing the existing one) \$\endgroup\$ – Blindman67 Mar 1 '18 at 5:22

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