0

Phew, even the question was hard to write. Here's the problem: I have a "game", more like a random simulator, which needs to choose a random action from an array of actions, like this one:

actions = [ Action1, Action2, Action3 ]

I have actions written as classes inheriting from the Action parent class:

function Action() {
    this.targets = [];
    this.used = [];
    this.execute = function(player) {
        doStuff();
        return whatever;
    };
}
//btw the below I've seen in a JS OOP tutorial but it doesn't work and I have to implement init() in every child action
Action.init = function(player) {
    var a = new this.constructor();
    return a.execute(player);
};
Action.checkRequirements = function() {
    return true;
};

Action1.prototype = new Action();
Action1.prototype.constructor = Action1;
function Action1 {
    this.execute = function(player) {
        doStuff();
        return whatever;
    }
}
Action1.init = function(player) {
    var a = new Action1();
    return a.execute(player);
}

So what I'm doing to execute an action and get its results is var foo = actions.getRandomVal().init(); (getRandomVal is a simple custom script that returns a random value from the array) It works well, creates the object instance which properly inherits all properties and methods, executes the exec() method and returns its results... but now I have a checkRequirements() method which I want to implement in like 10% of the 100+ actions I wish to do, and I want it to simply be inherited from the Action class so that when it is not implemented in the child class it simply returns true and I don't have an idea how. If I do var a = actions.getRandomVal(); and then a.checkRequirements(); it throws an exception that a.checkRequirements is not a function.

PS: this is a relatively small non-profit project for a (large) group of friends, I don't need it to work in every browser, it needs to work in Chrome and I can just tell them to use Chrome for it.

Deuxis
  • 227
  • 1
  • 11
  • `Action1.prototype = new Action();` is one of the reasons why [your OOP doesn't work](https://stackoverflow.com/questions/12592913/what-is-the-reason-to-use-the-new-keyword-here) – Bergi Apr 26 '17 at 16:30

2 Answers2

1

Since you only need to work with Chrome, I'd suggest to use ES6 class syntax which does all the inheritance properly, without the chance to mess up. This includes your Action1 constructor to inherit properties ("static class members") from the Action constructor as you'd expect.

class Action {
    constructor() {
        this.targets = [];
        this.used = [];
    }
    execute(player) {
        doStuff();
        return whatever;
    }
    static init(player) {
        var a = new this(); // no .constructor
        return a.execute(player);
    }
    static checkRequirements() {
        return true;
    }
}

class Action1 {
    execute(player) {
        doOtherStuff();
        return whateverelse;
    }
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Thank you, this works. I still don't know why the static methods weren't inherited when done the old way, but this is so much simpler and more elegant that I don't care. – Deuxis May 09 '17 at 21:19
  • 1
    @Deuxis Because in ES5, subclassing only did inheritance for the instances, not for the constructors (that hold the static properties). – Bergi May 09 '17 at 21:22
0

It looks to me like you're calling checkRequirements() on an instance:

a.checkRequirements();

But it's implemented statically:

Action.checkRequirements = function() {
    return true;
};

You probably want to bind this function to the prototype, so change the code above to this:

Action.prototype.checkRequirements = function() {
    return true;
};

Then when you want to override this in a derived type, like Action1, you can do this:

Action1.prototype.checkRequirements = function () {
    return (whatever);
}

As per comments, my guess is you want something like this...

// base Action type providing basic implementation
// Wrapped in an IIFE to prevent global scope pollution
// All functions are prototype bound to allow prototypical inheritance.
var Action = (function () {
    function Action() {
        this.targets = [];
        this.used = [];
    };

    Action.prototype.doStuff = function () {
        return;
    }

    Action.prototype.execute = function (player) {
        this.doStuff();
        return "whatever";
    }

    Action.prototype.checkRequirements = function () {
        return "foo";
    }

    return Action;
})();

var Action1 = (function () {
    Action1.prototype = new Action();
    Action1.prototype.constructor = Action1;

    function Action1() {
    }

    Action1.prototype.checkRequirements = function () {
        // Super call
        return Action.prototype.checkRequirements.call(this);
    }

    return Action1;
})();

var Action2 = (function () {
    Action2.prototype = new Action();
    Action2.prototype.constructor = Action2;

    function Action2() {
    }

    Action2.prototype.checkRequirements = function () {
        return "bar";
    }

    return Action2;
})();

// Set up array.
var array = [Action1, Action2];

// Create instances (this is where you would pick at random)
var a1 = new array[0]();
var a2 = new array[1]();
// var aofn = new array[rnd]();

// Tests
alert(a1.checkRequirements()); // Should "foo" because it called super (Action).
alert(a2.checkRequirements()); // Should "bar" because it's overridden.

Check it out on TypeScript Playground

Matthew Layton
  • 39,871
  • 52
  • 185
  • 313
  • This doesn't work, and there is no instance afaik, I never call `new` (and I don't even see **how** in the array situation). I'm choosing one from an array of classes and then calling its static method. It works if I implement the static `checkRequirements` in the child Action1 - it just doesn't get inherited from the parent Action. This is the same problem that made me implement `init()` in every child action even though it should be inherited from the parent. Oh well, I guess I'll just copy and paste `checkRequirements()` too. Leaving the question though, maybe someone will explain. – Deuxis Apr 26 '17 at 15:18
  • @Deuxis you are calling new from your `init()` function: `var a = new Action1();`. Let me update the answer...there will undoubtedly be an elegant solution. – Matthew Layton Apr 26 '17 at 15:21
  • @Deuxis out of curiosity, have you considered writing this with TypeScript first and seeing what the compiler produces? – Matthew Layton Apr 26 '17 at 15:31
  • @Deuxis see latest edit for alternative solution...or at least something to get you started. – Matthew Layton Apr 26 '17 at 15:43
  • Thank you for the help, but I first got frustrated and then used the class syntax from Bergi's answer because it's simpler and closer to my Java experience, and it worked, so I can't confirm or deny your answer. One thing though, I was calling init on randomly selected class - and that's exactly what wasn't working, I couldn't call the static method that should be inherited, had to define it for every child. – Deuxis May 09 '17 at 21:17