PDA

View Full Version : [1.1RC1] JsonReader and defaultValue



cumpa
11 Jul 2007, 1:33 AM
The defaultValue field property is not used when JsonReader cannot find a value with its mapping (thanks to Animal).

Consider this json object:

[{id: 1, firstName: 'Paolo', lastName: 'Rossi', country: {name: 'Florence', state: 'Italy'}},
{id: 2, firstName: 'Marco', lastName: 'Polo', country: null}
]

and this JsonReader:



Ext.data.Record.create([
{name: 'firstName'},
{name: 'lastName'},
{name: 'countryName', mapping: 'country.name', defaultValue:'N/A'},
{name: 'contryState', mapping: 'country.state', defaultValue:'N/A'}
])


The object with id 2 catch an exception instead using the defaultValue property.

Sorry for my english
Thanks
Matteo

Animal
11 Jul 2007, 1:51 AM
Here are the changes to fix this. My changes in bold:



Ext.override(Ext.data.JsonReader, {
getJsonAccessor: function(){
var re = /[\[\.]/;
return function(expr) {
try {
return(re.test(expr))
? new Function("rec", "defaultValue", "try{with (rec) return " + expr +
" || defaultValue;}catch(e){return defaultValue;}")
: function(rec, defaultValue){
try {
return rec[expr] || defaultValue;
} catch (e) {
return defaultValue;
}
};
} catch(e){}
return Ext.emptyFn;
};
}(),

/**
* Create a data block containing Ext.data.Records from an XML document.
* @param {Object} o An object which contains an Array of row objects in the property specified
* in the config as 'root, and optionally a property, specified in the config as 'totalProperty'
* which contains the total size of the dataset.
* @return {Object} data A data block which is used by an Ext.data.Store object as
* a cache of Ext.data.Records.
*/
readRecords : function(o){
/**
* After any data loads, the raw JSON data is available for further custom processing.
* @type Object
*/
this.jsonData = o;
var s = this.meta, Record = this.recordType,
f = Record.prototype.fields, fi = f.items, fl = f.length;

// Generate extraction functions for the totalProperty, the root, the id, and for each field
if (!this.ef) {
if(s.totalProperty) {
this.getTotal = this.getJsonAccessor(s.totalProperty);
}
if(s.successProperty) {
this.getSuccess = this.getJsonAccessor(s.successProperty);
}
this.getRoot = s.root ? this.getJsonAccessor(s.root) : function(p){return p;};
if (s.id) {
var g = this.getJsonAccessor(s.id);
this.getId = function(rec) {
var r = g(rec);
return (r === undefined || r === "") ? null : r;
};
} else {
this.getId = function(){return null;};
}
this.ef = [];
for(var i = 0; i < fl; i++){
f = fi[i];
var map = (f.mapping !== undefined && f.mapping !== null) ? f.mapping : f.name;
this.ef[i] = this.getJsonAccessor(map);
}
}

var root = this.getRoot(o), c = root.length, totalRecords = c, success = true;
if(s.totalProperty){
var v = parseInt(this.getTotal(o), 10);
if(!isNaN(v)){
totalRecords = v;
}
}
if(s.successProperty){
var v = this.getSuccess(o);
if(v === false || v === 'false'){
success = false;
}
}
var records = [];
for(var i = 0; i < c; i++){
var n = root[i];
var values = {};
var id = this.getId(n);
for(var j = 0; j < fl; j++){
f = fi[j];
var v = this.ef[j](n, f.defaultValue);
values[f.name] = f.convert(v);
}
var record = new Record(values, id);
record.json = n;
records[i] = record;
}
return {
success : success,
records : records,
totalRecords : totalRecords
};
}

methodz
12 Sep 2007, 9:22 AM
Did this not make it into 1.1 final?

Animal
12 Sep 2007, 1:07 PM
NO, perhaps it slipped off Jack's radar.

I do think it's a better approach.

jack.slocum
13 Sep 2007, 5:30 AM
Adding try/catches has the pontential to significantly degrade performance. I am not real comfortable with that change. :(

sjivan
13 Sep 2007, 8:13 PM
Adding try/catches has the pontential to significantly degrade performance. I am not real comfortable with that change. :(

Isn't this only when an exception is raised? Correct me if I'm wrong, but if I read the proposed change correctly, an overhead may possibly be incurred only if users choose to use the 'defaultValue' option. Regardless of the implementation (ofcourse if there's a more efficient impl., the better) , I think supporting this feature is useful, and adds consistency as the XmlReader suports this feature.

Sanjiv

Animal
13 Sep 2007, 11:50 PM
I think it's the setting up of the try/catch machinery which is an overhead. And for many accesses of an object, that would indeed be quite an overhead.

The "with(rec)" method does offer more flexibility in the kind of property mapping strings you can use this as your data object:



{root:[{id:1, firstName:"Fred", initial:"G", lastName:"Bloggs"}]}


You could use a full javascript expression in your mapping:



{name:fullName, mapping:"firstName + ' ' + initial + '. ' + lastName"}


So, we could just leave the try/catch out, as in the current JsonReader:



getJsonAccessor: function(){
var re = /[\[\.]/;
return function(expr) {
try {
return(re.test(expr))
? new Function("rec", "defaultValue", "with (rec) return " + expr + " || defaultValue;")
: function(rec, defaultValue){
return rec[expr] || defaultValue;
};
} catch(e){}
return Ext.emptyFn;
};
}(),

sjivan
14 Sep 2007, 2:31 AM
In Java having a try / catch adds no overhead until an exception is raised, in which case a performance hit is incurred in building the call stack trace. Not sure of the precise working of try/catch in Javascript.

As an aside, I remember you adding support for nested property types even for JsonReader? Do you know if that has been incorporated or are they any plans for incorporating them in Ext.

Thanks,
Sanjiv

Animal
14 Sep 2007, 3:57 AM
The current function for creating an accessor function in JsonReader is



getJsonAccessor: function(){
var re = /[\[\.]/;
return function(expr) {
try {
return(re.test(expr))

? new Function("obj", "return obj." + expr)
: function(obj){
return obj[expr];
};
} catch(e){}
return Ext.emptyFn;
};
}(),


The code for "complex" expressions (ie involving "[" or ".") is in bold.

So for record elements like



{root:[{id: 1, name: {first: "Fred", initial:"G", last:"Bloggs"}}]}


Your field def could be



{name: "name", mapping:"name.first + ' ' + obj.name.initial + ' ' + obj.name.last"}


It's just that you need to reference the passed object explicitly, rather than having it assumed using the "with()" construct.