add a toString to StatefulModel

[ben hockey]

trying to use StatefulModel?, i've found it a little bit annoying when trying to display values of leaf nodes to have to use .value - ie var str = model.node.value rather than var str = model.value. i think a toString on StatefulModel? could be useful. something like this...

toString: function () {
        return this.value === "" ? this.data.toString() : this.value.toString();
}

[ben hockey]

hmm... update to the toString

toString: function () {
    return this.value === "" && this.data ? this.data.toString() : this.value.toString();
}

[rahul]

Seems OK, though really what you're after is the value at a given node in the model, so perhaps toValue() is a better name.

[ben hockey]

i've given some thought to what it is i'm trying to achieve with this and what i want is for nodes that represent primitive values (leaf nodes) to be able to act as those primitives without having to reference the value property. there is already good support for "hiding" the model for non-primitives (arrays and objects) such that you can reference model.foo.bar to get to the bar node but then you need to use bar.value to get its value.

toString and valueOf are special methods in javascript (see ​http://www.adequatelygood.com/2010/3/Object-to-Primitive-Conversions-in-JavaScript for a good explanation) and both of these would be useful for "hiding" the underlying structure of a leaf node. the idea being that if i have model.foo.bar that represents data.foo.bar then i can easily convert existing data.foo.bar code (in my case, mostly templates that are doing string substitution) to model.foo.bar code without needing to change everything to model.foo.bar.value. there will probably be some exceptions that need to be handled but this convenience provided by toString and valueOf would go a long way to simplifying the transition imo.

if i understand the way that StatefulModel? stores data then i think the following valueOf and toString methods would work for all nodes in a model regardless of whether they represent a primitive.

valueOf: function () {
    // return the data that this node represents
    return this.data;
},
toString: function () {
    // return the toString of the data that this node represents
    return this.data.toString()
}

i'm not locked into these implementations if i've misunderstood the structure of the internals of StatefulModel? or you have alternative implementations for toString and valueOf but just thought that this kind of convenience might be useful.

[Ed Chatelain]

this.data holds the last saved model state, so if a change has been made but not saved it will not be set into this.data, so I don't think the latest proposal for valueOf() and toString() functions are right. Actually I think your previous proposal for toString() is good, and valueOf is actually the same as the toPlainObject() function.

Should we create a valueOf() function that just calls this.toPlainObject() inorder to have a valueOf function? I guess another option is to rename toPlainObject() to valueOf, but that would hit a lot of tests, or we could not have support for valueOf().

I think we should add the valueOf() and have it call toPlainObject(). And I think we should add the toString() as well.

I will create a patch for it, but if anyone objects I can rework the patch.

[Ed Chatelain]

Patch to add toString() and valueOf() to StatefulModel?, and a test update to verify it.

[Ed Chatelain]

Since there were no objections to my proposal above, I made this update along with others via this changeset: In [26567] which was made for ticket #13773

[ben hockey]

the code seems fine. you could make some changes to the tests to properly test what is intended for valueOf and toString. the calls to these are meant to be implicit so tests should be more like this:

valueOfTest = new dojox.mvc.StatefulModel({ data: 42 });
doh.is(42,valueOfTest.data,"valueOfTest.data is wrong");
doh.is(42,valueOfTest.value,"valueOfTest.value is wrong");
doh.is(42,+valueOfTest,"valueOfTest.valueOf() is wrong");
doh.is("42",'' + valueOfTest,"valueOfTest.toString() is wrong");

toStringTest = new dojox.mvc.StatefulModel({ data: 'abc' });
doh.is('abc',toStringTest.data,"toStringTest.data is wrong");
doh.is('abc',toStringTest.value,"toStringTest.value is wrong");
doh.is(NaN,+toStringTest,"toStringTest.valueOf() is wrong");
doh.is('abc','' + toStringTest,"toStringTest.toString() is wrong");

sorry for not looking at this sooner. the implementation seems fine so i don't think its a big deal if the tests aren't updated. however, the changes would be a demonstration of the intended usage for anyone looking at the tests.

[Ed Chatelain]

Thanks for the tests, I think I will create a new testcase for this and commit it next time I have an mvc update.

[Ed Chatelain]

I have updated a test case to include your test above, with this update: ​http://bugs.dojotoolkit.org/changeset/26660/dojo So I think this ticket can be closed, but unfortunately I don't have the permissions to close it.