CODEWITHSUNDEEP
javascript
Tyshawn
Tyshawn

Reputation: 39

JavaScript: calling private method's property on class object

For the class Engine, "start" and "stop" are two public methods which internally calls "ignitionOn" and "ignitionOff". After "start" is called the private method sets a variable "ignitionIndicator" to "true" on the engine instance and "stop" sets it to "false".

However that is not happening. Something is going wrong since the "ignitionIndicator" value is always "undefined". I have to keeping the following in mind.

(i) Visibility of the methods should be as it is.

(ii) I CAN NOT set the variable directly from the public methods. The variable should be only and only set from within the private method.

function Engine() {
  function EngineConstructor() { };

  // publicly accessible methods
  EngineConstructor.prototype.start = function() {
    ignitionOn();
  };

  EngineConstructor.prototype.stop = function() {
    ignitionOff();
  };

  // private methods 
  function ignitionOn() {
    // does other things and sets this to true
    this.ignitionIndicator = true;
  };

  function ignitionOff() {
    // does other things and sets this to false
    this.ignitionIndicator = false;
  };

  return new EngineConstructor();  
};
var e = new Engine();
e.start();
e.ignitionIndicator // undefined, should have been true
e.stop();
e.ignitionIndicator // undefined, should have been false

Upvotes: 0

Views: 103

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074138

You'd need to call those methods such that this refers to an instance, for instance via Function#call or Function#apply, e.g.:

EngineConstructor.prototype.start = function() {
    ignitionOn.call(this);
};

Or alternately, just pass the instance as an argument (procedural style):

EngineConstructor.prototype.start = function() {
    ignitionOn(this);
};

// ...

function ignitionOn(instance) {
    // does other things and sets this to true
    instance.ignitionIndicator = true;
}

The pattern you're using in that code is very odd. You have a function, Engine, which you use as a constructor (via new) but which isn't a constructor (it returns something other than the instance created by the new operator), and the object it returns is unrelated to the Engine function — instead, it's an instance created by EngineConstructor. Worse, it creates a whole new EngineConstructor every time you call it. This is hugely inefficient; if you want to recreate all the methods for every instance, you can do so much more simply.

But I'd just revisit the entire structure so that you do get method reuse. If the goal is to have a constructor function which has access to private methods from prototype methods, the revealing module pattern (applied here to a single constructor) is the usual way to do that:

var Engine = function() {
    // The constructor
    function Engine() {
    }

    // Publicly accessible methods
    Engine.prototype.start = function() {
        ignitionOn.call(this);
    };

    Engine.prototype.stop = function() {
        ignitionOff.call(this);
    };

    // Private methods 
    function ignitionOn() {
        // does other things and sets this to true
        this.ignitionIndicator = true;
    }

    function ignitionOff() {
        // does other things and sets this to false
        this.ignitionIndicator = false;
    }

    return Engine;
}();
var e = new Engine();
e.start();
snippet.log(e.ignitionIndicator);
e.stop();
snippet.log(e.ignitionIndicator);
<!-- Script provides the `snippet` object, see http://meta.stackexchange.com/a/242144/134069 -->
<script src="http://tjcrowder.github.io/simple-snippets-console/snippet.js"></script>

Side note: Function declarations don't have a ; after them. ; is a statement terminator, and function declarations (such as those for ignitionOn and ignitionOff) are not statements.

Upvotes: 3

Related Questions