Support shorthand getter setters ~5

[justinbmeyer]

Example:

Todo.List = can.DefineList.extend({
    "*": Todo,
    get complete(){
            return this.filter({complete: true});
    },
    get active(){
            return this.filter({complete: false});
    },
    get allComplete(){
            return this.length === this.complete.length;
    },
    updateCompleteTo: function(value){
        this.forEach(function(todo){
            todo.complete = value;
            todo.save();
        });
    },
    get saving(){
            return this.filter(function(todo){
                return todo.isSaving();
            });
    }
});

[justinbmeyer]

The problem here is that getters in this fashion can't be defined with arguments and setters can only accept one argument.

An alternative would be supporting something like:

Todo.List = can.DefineList.extend({
    "*": Todo,
    "complete get": function(){
            return this.filter({complete: true});
    },
    "active get": function(){
            return this.filter({complete: false});
    },
    "allComplete get": function(){
            return this.length === this.complete.length;
    },
    updateCompleteTo: function(value){
        this.forEach(function(todo){
            todo.complete = value;
            todo.save();
        });
    },
    "saving get": function(){
            return this.filter(function(todo){
                return todo.isSaving();
            });
    }
});

[justinbmeyer]

QUnit.test("getter setters work (#56)", 2, function(){
    var MyMap = define.Constructor({
        first: "string",
        last: "string",
        get fullName() {
            return this.first +" "+ this.last;
        },
        set fullName(newVal) {
            QUnit.ok(true, "setter called");
            var parts = newVal.split(" ");
            this.first = parts[0];
            this.last = parts[1];
        }
    });



    var p = new MyMap({first: "J", last: "M"});


    p.on("fullName", function(ev, newVal){
        QUnit.equal(newVal, "B M");
    });

    p.fullName = "B M";



});

[gsmeets]

The shorthand doesn't add that much value imo.

[justinbmeyer]

@gsmeets IMO, it feels awfully repetitive to write:

Todo.List = can.DefineList.extend({
    "*": Todo,
    complete: {
        get: function(){
            return this.filter({complete: true});
        }
    },
    active: {
        get: function(){
            return this.filter({complete: false});
        }
    },
    allComplete: {
        get: function(){
            return this.length === this.complete.length;
        }
    },
    updateCompleteTo: function(value){
        this.forEach(function(todo){
            todo.complete = value;
            todo.save();
        });
    },
    saving: {
        get: function(){
            return this.filter(function(todo){
                return todo.isSaving();
            });
        }
    }
});

2 extra lines (and indentation) per getter. 10 characters saved in the ES5 getter/setter version.

The string version ("something get") could also make defining initial values a bit easier:

{
  "count value": 0
}

[gsmeets]

I think saving a few lines doesn't add up against the possibility of introducing bugs and reducing readability when you can combine the same property on multiple indexers on your viewModel. (Like in your get/set fullName example, but the same applies if you use strings for it.)

If you want to go forward with this I'd at least not allow or warn when you detect multiple uses for the same name. In that case you should be using the full version and not the shorthand imo. Just merging those properties is debugging disasters waiting to happen. But that's just my 2 cts.

If this lands I'd prefer it landing as a plugin. But I can see how that might not be trivial, as it hits a bit on the core of can define.

[justinbmeyer]

btw, the compiled formal definitions are always on the Constructor._define in case you need debugging for something like this.

[gsmeets]

I'm just thinking more about my juniors here and the weird issues they can run across. They're full well able to get stuck and wait an hour before asking help. ;)

[justinbmeyer]

@gsmeets I appreciate the concern, but after discussing this in the DoneJS Contributors meeting, we felt that the ES5 getter / setter approach was worth it. It's possible someone tries this syntax anyway, and it would be great to support it. As an advanced user, I will use it all the time.

The docs for this will be out soon.