10
\$\begingroup\$

I am creating my first HTML5 game using JavaScript and Phaser framework. I have a class (or more accurately a prototype) called MajorPoint that represents the starting point and the end point of the game. I initially put those two together, as they only differed from each other by the visible frame. Now as the game is more fleshed out though, the endpoint has got a few more rules that aren't needed by the starting point.

Should I now split this into two? I'm not sure what to do with it, because if I split this into two (StartPoint and EndPoint), those two prototypes would have a lot (relatively speaking) of identical code between each other. On the other hand, now there is almost as much code that is only needed by half of the objects.

The objects will not change their state during the game, so they stay as they are created the whole time they exist.

var MajorPoint = function(game, x, y, state, callback, callbackContext) {
  Phaser.Sprite.call(this, game, x, y, 'start-end', 0);
  this.state = state;
  this.anchor.setTo(0.5, 0.7);
  this.scale.setTo(0.5, 0.5);

  //If state is 'end' but end is not reachable, hides this object.
  //If state is 'end' and end is reachable, enables input for this object.
  if (state === 'end') {
    this.frame = 1;
    if (!game.data.isEndReachable()) {
      this.visible = false;
    } else {
      this.inputEnabled = true;
      if (callback !== undefined) {
        this.events.onInputDown.add(callback, callbackContext);
      }
    }
  }
};

MajorPoint.prototype = Object.create(Phaser.Sprite.prototype);
MajorPoint.prototype.constructor = MajorPoint;
\$\endgroup\$
2
  • \$\begingroup\$ I personally think your class is doing too much - It is maintaining it's size and shape but also the logic required for it to work. You should instead have the class called MajorPoint that denotes a point of importance on the map and have event handlers on there. Then, at the creation of the end MajorPoint, attach handlers specifically for that point to that instance. Not posting as an answer because I don't fully understand your problem space and this might be impossible. \$\endgroup\$
    – Dan
    Mar 7 '15 at 13:39
  • \$\begingroup\$ Also the only solution I can think of is using an entity-component system but that is definitely overkill for you \$\endgroup\$
    – Dan
    Mar 7 '15 at 13:52
3
\$\begingroup\$

You can create EndPoint as a more specific version of MajorPoint and move the extra logic there.

var MajorPoint = function(game, x, y, state, callback, callbackContext) {
  Phaser.Sprite.call(this, game, x, y, 'start-end', 0);
  this.state = state;
  this.anchor.setTo(0.5, 0.7);
  this.scale.setTo(0.5, 0.5);
};

MajorPoint.prototype = Object.create(Phaser.Sprite.prototype);
MajorPoint.prototype.constructor = MajorPoint;

var EndPoint = function(game, x, y, state, callback, callbackContext) {
  MajorPoint.call(this, game, x, y, 'start-end', 0);

  //If end is not reachable, hides this object.
  //If end is reachable, enables input for this object.
  this.frame = 1;
  if (!game.data.isEndReachable()) {
    this.visible = false;
  } else {
    this.inputEnabled = true;
    if (callback !== undefined) {
      this.events.onInputDown.add(callback, callbackContext);
    }
  }
};

EndPoint.prototype = Object.create(MajorPoint.prototype);
EndPoint.prototype.constructor = EndPoint;

If callback is only valid when it's omitted or a function, you can drop the !== undefined and just test that it's truthy. This will avoid setting an invalid callback when someone accidentally passes null.

if (callback) {
  ...
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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