Opened 10 years ago
Closed 9 years ago
#13250 closed enhancement (fixed)
add a toString to StatefulModel
Reported by: | ben hockey | Owned by: | rahul |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | DojoX MVC | Version: | 1.7.0b1 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
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(); }
Attachments (1)
Change History (11)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
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.
comment:3 Changed 10 years ago by
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.
comment:4 Changed 10 years ago by
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.
Changed 10 years ago by
Attachment: | 13250-mvc.patch added |
---|
Patch to add toString() and valueOf() to StatefulModel?, and a test update to verify it.
comment:5 Changed 9 years ago by
comment:6 Changed 9 years ago by
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.
comment:7 Changed 9 years ago by
Thanks for the tests, I think I will create a new testcase for this and commit it next time I have an mvc update.
comment:8 Changed 9 years ago by
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.
comment:9 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:10 Changed 9 years ago by
Milestone: | tbd → 1.7 |
---|
hmm... update to the toString