Retired

Class extension model with mixin ability. (retired)

301 of 303.blue
Description
Loading description...
Fundamentals
View
AllIssues1QuestionsSuggestions1Show Resolved
  • Please sign in or sign up to leave a comment.
  • FArekkusu Avatar
     
    SUPER:
function baseClass(config){
  this.config = config;
  this.setConfig = function(value){};
}

MIXIN:
function observableMixin(){
  this.listeners = {};
  this.addListener = function(name, callback){
    if(!this.listeners[name]) this.listeners[name] = [];
    this.listeners[name].push(callback);
  }
  this.removeListener = function(name, callback){
    this.listeners[name] = this.listeners[name].filter(
      function(fn){ 
        return fn != callback;
        });
  }
}

    TypeError: c.hasListener is not a function
    at it
    at begin
    at it
    at describe
    at /runner/frameworks/javascript/cw-2.js:152:11
    at Promise._execute
    at Promise._resolveFromExecutor
    at new Promise
    at describe
    at /home/codewarrior/index.js:49:5
    at /home/codewarrior/index.js:96:5
    at Object.handleError

    There's no hasListener method anywhere, and no explanation of what the tests are trying to do - how am I even supposed to debug this?

    Edit: turns out there's a observableMixin.prototype.hasListener method, but again, if your solution is wrong and doesn't add this method to the created class, you will likely not be able to tell what's wrong.

  • computerguy103 Avatar

    I'd recommend adding some example code in the description. Show how you expect the extend function to be invoked, with or without mixin and scope.

    Also, the function's argument names in the description doesn't match up with the argument names in the solution setup:

    extend('ChildClass', BaseClass, EventDispatcher, scope);
function extend(className, superClass, mixin, scope)

    You should change one or both so that the argument names are consistent. The latter names are clearer IMHO.

    Then, change your bullet list so that it has a bullet item for each argument, describing the type (string, function, object), what it represents, and identifying the arguments which were optional.

  • .blue Avatar

    What else should I improve in this kata? :)

  • christianhammer Avatar

    According to your own solution the mixin is optional. This is not stated in the description - nor is tested for. My solution, much in line of your own was accepted though it will fail when mixin is not set.

    • .blue Avatar

      Hey christianhammer, Congrats! you are the first one to solve it anyway :) The mixin was not declared as being optional so it is not tested for as being optional., where is the problem? :) My solution took care about the eventuality of providing some invalid value for the mixin by instinct :))

      However, I find the problem nice and pretty much easy to be solved but I see that people don't like it, so I will probably quit the idea :)

      Thanks for taking the time to solve and comment it!

    • .blue Avatar

      As a last touch I have added tests for the mixin and specified it as being optional ;)

    • christianhammer Avatar

      Hi .blue. The downvotes you see are automatically set when you receive a "Minor issues" and/or "Major Issues". So don't be too discouraged by that count - I personally find your kata illustrative (especially if you have never worked with extend and 'classical' inheritance in JavaScript).

      Argh - "mixin" being optional will invalidate my solution... Wonder if I can remedy it ;-)

    • .blue Avatar

      Oh I see! Thank you for clearing that out for me :)

      I am sure you can remedy it :D

  • .blue Avatar

    Hey laoris,

    I have added some friendly comments to my test cases. Hopefully I didn't make them too obvious now?!

    Thank you!

  • .blue Avatar

    Thanks laoris for reviewing my first kata :)

    1. One would suppose that the ChildClass will be defined in the same scope as the BaseClass, which is provided, and the same scope the extend function is called, so it is no need to return a reference. That was intended.

    2. Test cases are testing if the subclass is correctly generated from the base class and mixin. I will think of some comments in there that would not spoil the subject :)

    Thank you!

  • laoris Avatar

    Some problems I see:

    1. Specifying the child class name as a parameter is a weird way to name the class. I have to assume that the extend function is expected to put the subclass in the global scope, but this isn't specified in the kata. It would be better for the extend function to return a reference to the child class' constructor so that the caller can decide the scope:

      var ChildClass = extend(BaseClass, EventDispatcher);
this['ChildClass'] = extend(BaseClass, EventDispatcher);

    2. The test cases provide no indication of what they are testing. Failing the second test just gives me the error message: "Test Failed: Expected: true, instead got: false". The tests in the provided test fixture are better because they have messages indicating what is wrong, and it'd be better if the test cases would do the same.