PDA

View Full Version : Ext.grid.GridView should use interface methods of Record for rendering fields


YetAnotherFrank
07-06-2009, 08:50 AM
Ext.grid.GridView should use the interface methods of Ext.data.Record for rendering the record fields.

Ext.data.Record provides a public method get( String name ) : Object to access the values of its fields. However the GridView does not use this interface but accesses the data directly like this: record.data[name].

This is a problem if you write your own Record class that provides its own get method, because the view will not use this get method.

For example I wrote a Record class that loads its fields dynamically and attaches an event listener to each record that is rendered by the view, so that the view will be notified if the value of a field changes.

I had to patch the doRender method of the GridView to call my get method like this:

p.value = c.renderer(r.get(c.name), p, r, rowIndex, i, ds);instead of:

p.value = c.renderer(r.data[c.name], p, r, rowIndex, i, ds);

Condor
07-06-2009, 09:01 AM
There are lots of other places this would also have to be fixed (Store.sortData comes to mind).

This is one of the reasons why I created the CalcRecord user extension.

jgarcia@tdg-i.com
07-06-2009, 10:03 AM
Isn't accessing a reference directly faster than calling a method, which then returns the reference?

Condor
07-06-2009, 10:07 AM
Exactly, that is the main reason it doesn't use the get method.

(why is there even a get method?)

jgarcia@tdg-i.com
07-06-2009, 10:09 AM
Exactly, that is the main reason it doesn't use the get method.

(why is there even a get method?)

I suppose that it's there because it's cleaner easier for people to learn to "get" a property from the record?

YetAnotherFrank
07-07-2009, 02:50 AM
There are lots of other places this would also have to be fixed (Store.sortData comes to mind).

This is one of the reasons why I created the CalcRecord user extension.

Sounds like I should indeed have a look at your CalcRecord extension! Thanks for the pointer, Condor!

Isn't accessing a reference directly faster than calling a method, which then returns the reference?

Exactly, that is the main reason it doesn't use the get method.

(why is there even a get method?)

Of course, using a property instead of a getter function is faster, but I think in this case, it should not (only) be a question of efficiency. I am convinced that code accessing the Record data is not executed frequently enough to make a measurable difference (I reckon we are talking about hundreds of calls per second). If you have already profiled this option and found it too costly, please let me know! For sorting, it might be necessary to cache the relevant cell data.

I understand the get method as the primary interface to access a Record's cell data---however, as there are neither interfaces nor private members in JavaScript, this is hard to tell.

Anyway, Record#get(name) being consistently used for reading, not Record#data[name], would open many new options to calculate Record data on the fly (saving memory) or fetch it on demand, and should really not have a big impact on efficiency, as long as the get function does nothing expensive.

evant
07-07-2009, 03:05 AM
Agreed, I think it should use the interface method. We'll look at cleaning this up.

YetAnotherFrank
07-07-2009, 04:09 AM
Great! Thanks!

aconran
07-10-2009, 10:44 PM
In the raw underlying code which builds the UI interface markup we've tried to optimize it as much as possible. As Condor mentioned above the reason that it accesses the data directly is because it *much* faster.

Take a benchmark of accessing data directly out of an object vs retrieving it with a function; unfortunately there *is* a performance hit when you look at it across all the browsers.

It's because of this performance hit that we haven't built something akin to java's StringBuilder which would decide whether to do an array join or concatonate strings based on whether we are in IE or FF.

The performance hit of calling the function is slower than one would expect :-/

YetAnotherFrank
07-13-2009, 03:39 AM
Okay, if it is really a performance optimization, what about the following suggestion:

Check once for each record whether it is an instance of exactly Ext.data.Record.
If so, always use the data field directly.
Otherwise: use the more general contract, the interface method #get().Like so:

var useDataDirectly = r.constructor===Ext.data.Record;
...
p.value = c.renderer(useDataDirectly ? r.data[c.name] : r.get(c.name), p, r, rowIndex, i, ds);

This would be like "manual inlining"...