PDA

View Full Version : [CLOSED-480] New "getTotal" bug in 0.98 (worked in 0.97)



Steffen Hiller
28 Oct 2010, 10:45 AM
In 0.98 you can't instantiate a Reader directly.

In the Twitter example change TwitterProxy.js:23 from


reader: {
type: 'json',
root: 'results'
}

to


reader: new Ext.data.JsonReader({
root: 'results'
})


This results in:
Uncaught TypeError: Object [object Object] has no method 'getTotal'

edspencer
28 Oct 2010, 11:49 AM
Interesting - I didn't anticipate that people would want to use the longer syntax (new Ext.data.JsonReader vs a simple config object). All of the examples use the config instead of instantiating directly. Is there a reason you're doing this instead?

Steffen Hiller
28 Oct 2010, 11:56 AM
No, just habit. Maybe you could argument that the reader config itself stays cleaner. ;)

But wouldn't it break convention if you could instantiate some classes and some classes not??

edspencer
28 Oct 2010, 12:01 PM
The various classes do some magic internally that favor the config approach - for example a Reader needs a Model (or a set of fields) to be able to read, but if you pass in config the Store automatically forwards the Model it was configured with to the Reader. Same deal with Proxy - if it is specified in the Store config it is automatically given the Model object.

I'm not aware of any cases where there is benefit in instantiating directly - config is preferred. It also gives us opportunities to use lazy instantiation (though we don't do that currently)

Steffen Hiller
28 Oct 2010, 12:28 PM
Ok, there has never been an issue with that in Ext JS and Sencha Touch til 0.98. In the Ext JS docs there's an example with the instantiation of a Ext.data.JsonReader. So, for me that was unexpected behaviour == bug.

Now, if you say you will be deprecating that approach in the future, that's another thing. ;)

I totally support enforcing best practices and can't see any advantages in instantiating reader class either.

So, we can close this thread. (No, see my geistesblitz at the end.)

One suggestion though:
Would be nice if the type name of each reader and proxy class would be displayed somewhere in the top of each classes api docs page.
If you know it you know it of course, but new users might wonder if it's type: 'script-tag' or 'scripttag' or 'scripttagproxy'.

...
Now while typing I remember why I instantiated the Reader class directly:
I had a custom Reader first, and out of habit and laziness, a didn't register a type for it, but just made up a classname and instantiated it, which saved me a couple of characters eventually. ;)
So, if this is not a bug here, in the future, you *have* to register a type to use your own reader.
It's something not very natural since you have the choice in other parts of sencha touch, like panels.

And that's what doesn't feel right, you can instantiate some classes, but others not.

edspencer
28 Oct 2010, 12:43 PM
Fair enough on all counts. I think I can make Store a little more adaptable and call yourReaderInstance.setModel with the Store's model. I think that will solve the issue and allow instantiation of Proxies/Readers/Writers to work as well as config. Hopefully I can have that in place before the next release.

As far as documentation goes - it's a continual work in progress. I'm trying hard to get the data docs as good as possible but there's always more to do! Will add more examples for the next release along with an xtype listing.

Steffen Hiller
28 Oct 2010, 1:09 PM
Cool.

Regarding docs, maybe this here is a quick fix of the most annoying bug I'm experiencing: http://www.sencha.com/forum/showthread.php?112161-OPEN-419-Online-API-docs-search-Ext-JS-docs-instead-of-Sencha-Touch-docs&highlight=search+docs
Search e.g. for 'panel' and you'll find borderlayout stuff in the sencha touch docs. ;)

Also, I fully support defining all store and reader stuff in the model. Was thinking about that today and that is definitely the Rails way. (which I like)
The reason why I can't follow that approach in my app is that it depends on an external API.
So the way I would solve that with ActiveResource is defining an abstract class 'User' and then two specific subclasses 'FollowingUser' and 'FollowedUser' with the respective URLs (it's not just parameters).
And I think I can and probably should do the same in ST.

So thinking about it further, I wonder if there will be any need in the future, to instantiate stores, readers and proxies outside of a model.

Even


Ext.DataView({
store: new Ext.data.Store({
model: 'Image'
});
})

could be cut to


Ext.DataView({
model: 'Image'
})

and MVC would be even cleaner in ST or Ext JS 4. Because till now it's more MSVC (with MVC with Store).

But then, since it worked until 0.97, maybe the best for now is just to fix that 'bug'.

edspencer
28 Oct 2010, 1:24 PM
I think it'll always be MVSC/MSVC/SMVC/whatever - Stores are a useful abstraction that make sense on the client side. There is no one to one mapping between a Model and a Store though so your second syntax isn't quite right - I think we'll have something like this though:



new Ext.DataView({
store: {
model: 'Image'
}
});


Saves a few keystrokes :)

Steffen Hiller
28 Oct 2010, 1:32 PM
Aah, well, habits. ;)

Regarding stores I'm not sure if the extra abstraction is really needed. Listeners would be the only thing which would come to mind. But even regarding that I was going to suggest one day to relay the store events to the owner container and add the 'store' prefix. ;)

Steffen Hiller
28 Oct 2010, 1:54 PM
Wait, not to the owner container ... to the model that would be, and without the need of a "store" prefix.

Like:



Ext.regModel('Tweet', {
listeners: {
beforeload: function () {}
}
});


Is that already possible?
Would be nice I think, and would eliminate the need of an abstraction of the store in my eyes.

Just as you do before_save, etc. in a Rails Model.






Regarding stores I'm not sure if the extra abstraction is really needed. Listeners would be the only thing which would come to mind. But even regarding that I was going to suggest one day to relay the store events to the owner container and add the 'store' prefix. ;)