[FIX] do not use for...in to iterate on arrays, avoid using it to iterate on object as well. Use _.each in either cases. See doc for rationale
bzr revid: xmo@openerp.com-20110617115226-21meg23d0tg43wgv
This commit is contained in:
parent
ff50ec6407
commit
8ad063965d
|
@ -39,17 +39,18 @@ openerp.base_graph.GraphView = openerp.base.Controller.extend({
|
||||||
this.group_field = '';
|
this.group_field = '';
|
||||||
this.orientation = this.fields_view.arch.attrs.orientation || '';
|
this.orientation = this.fields_view.arch.attrs.orientation || '';
|
||||||
|
|
||||||
for(var fld in this.fields_view.arch.children) {
|
_.each(this.fields_view.arch.children, function (field) {
|
||||||
if (this.fields_view.arch.children[fld].attrs.operator) {
|
if (field.attrs.operator) {
|
||||||
this.operator.push(this.fields_view.arch.children[fld].attrs.name);
|
this.operator.push(field.attrs.name);
|
||||||
}
|
}
|
||||||
else if (this.fields_view.arch.children[fld].attrs.group) {
|
else if (field.attrs.group) {
|
||||||
this.group_field = this.fields_view.arch.children[fld].attrs.name;
|
this.group_field = field.attrs.name;
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
this.chart_info_fields.push(this.fields_view.arch.children[fld].attrs.name);
|
this.chart_info_fields.push(field.attrs.name);
|
||||||
}
|
}
|
||||||
}
|
}, this);
|
||||||
|
|
||||||
this.operator_field = this.operator[0];
|
this.operator_field = this.operator[0];
|
||||||
if(this.operator.length > 1){
|
if(this.operator.length > 1){
|
||||||
this.operator_field_one = this.operator[1];
|
this.operator_field_one = this.operator[1];
|
||||||
|
@ -80,53 +81,53 @@ openerp.base_graph.GraphView = openerp.base.Controller.extend({
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
schedule_chart: function(result) {
|
schedule_chart: function(results) {
|
||||||
this.$element.html(QWeb.render("GraphView", {"fields_view": this.fields_view, "chart": this.chart,'view_id': this.view_id}));
|
this.$element.html(QWeb.render("GraphView", {"fields_view": this.fields_view, "chart": this.chart,'view_id': this.view_id}));
|
||||||
this.opration_fld = {};
|
this.opration_fld = {};
|
||||||
if (result.length){
|
if (results.length){
|
||||||
for(var res in result) {
|
_.each(results, function (result) {
|
||||||
for(var fld in result[res]) {
|
_.each(result, function (field_value, field_name) {
|
||||||
if (typeof result[res][fld] == 'object') {
|
if (typeof field_value == 'object') {
|
||||||
result[res][fld] = result[res][fld][result[res][fld].length - 1];
|
result[field_name] = field_value[field_value.length - 1];
|
||||||
}
|
}
|
||||||
if (typeof result[res][fld] == 'string'){
|
if (typeof field_value == 'string'){
|
||||||
if (this.all_fields[fld]['selection']){
|
var choices = this.all_fields[field_name]['selection'];
|
||||||
for (var i in this.all_fields[fld]['selection']){
|
_.each(choices, function (choice) {
|
||||||
if(result[res][fld] == this.all_fields[fld]['selection'][i][0]){
|
if(field_value == choice[0]){
|
||||||
result[res][fld] = this.all_fields[fld]['selection'][i];
|
result[field_name] = choice;
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
});
|
||||||
}
|
}
|
||||||
}
|
}, this);
|
||||||
}
|
}, this);
|
||||||
|
|
||||||
for (var j in result){
|
_.each(results, function (result) {
|
||||||
var gen_key = result[j][this.chart_info_fields]+"_"+result[j][this.group_field];
|
var gen_key = result[this.chart_info_fields]+"_"+result[this.group_field];
|
||||||
if (this.opration_fld[gen_key] == undefined){
|
if (this.opration_fld[gen_key] == undefined){
|
||||||
var map_val = {};
|
var map_val = {};
|
||||||
map_val[this.operator_field] = result[j][this.operator_field];
|
map_val[this.operator_field] = result[this.operator_field];
|
||||||
if (this.operator.length > 1){
|
if (this.operator.length > 1){
|
||||||
map_val[this.operator_field_one] = result[j][this.operator_field_one];
|
map_val[this.operator_field_one] = result[this.operator_field_one];
|
||||||
}
|
}
|
||||||
map_val[this.chart_info_fields] = result[j][this.chart_info_fields];
|
map_val[this.chart_info_fields] = result[this.chart_info_fields];
|
||||||
if (this.group_field){
|
if (this.group_field){
|
||||||
map_val[this.group_field] = (typeof result[j][this.group_field] == 'object')?result[j][this.group_field][1]:result[j][this.group_field];
|
map_val[this.group_field] = (typeof result[this.group_field] == 'object')?result[this.group_field][1]:result[this.group_field];
|
||||||
}
|
}
|
||||||
this.opration_fld[gen_key] = map_val;
|
this.opration_fld[gen_key] = map_val;
|
||||||
}else{
|
} else {
|
||||||
map_val = this.opration_fld[gen_key];
|
map_val = this.opration_fld[gen_key];
|
||||||
map_val[this.operator_field] += result[j][this.operator_field];
|
map_val[this.operator_field] += result[this.operator_field];
|
||||||
if (this.operator.length > 1){
|
if (this.operator.length > 1){
|
||||||
map_val[this.operator_field_one] += result[j][this.operator_field_one];
|
map_val[this.operator_field_one] += result[this.operator_field_one];
|
||||||
}
|
}
|
||||||
this.opration_fld[gen_key] = map_val;
|
this.opration_fld[gen_key] = map_val;
|
||||||
}
|
}
|
||||||
}
|
}, this);
|
||||||
|
|
||||||
var graph_data = [];
|
var graph_data = [];
|
||||||
for (var op in this.opration_fld){
|
_.each(this.opration_fld, function (column_data) {
|
||||||
graph_data.push(this.opration_fld[op]);
|
graph_data.push(column_data);
|
||||||
}
|
});
|
||||||
|
|
||||||
if(this.chart == 'bar') {
|
if(this.chart == 'bar') {
|
||||||
return this.schedule_bar(graph_data);
|
return this.schedule_bar(graph_data);
|
||||||
|
@ -136,14 +137,14 @@ openerp.base_graph.GraphView = openerp.base.Controller.extend({
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
schedule_bar: function(result) {
|
schedule_bar: function(results) {
|
||||||
var self = this;
|
var self = this;
|
||||||
var view_chart = '';
|
var view_chart = '';
|
||||||
var xystr = {};
|
var xystr = {};
|
||||||
var xyname = {};
|
var xyname = {};
|
||||||
var res = [];
|
var res = [];
|
||||||
this.group_list = [];
|
this.group_list = [];
|
||||||
var newkey = '';
|
var newkey = '', newkey_one;
|
||||||
|
|
||||||
var COLOR_PALETTE = ['#cc99ff', '#ccccff', '#48D1CC', '#CFD784', '#8B7B8B', '#75507b', '#b0008c', '#ff0000', '#ff8e00', '#9000ff', '#0078ff', '#00ff00', '#e6ff00', '#ffff00',
|
var COLOR_PALETTE = ['#cc99ff', '#ccccff', '#48D1CC', '#CFD784', '#8B7B8B', '#75507b', '#b0008c', '#ff0000', '#ff8e00', '#9000ff', '#0078ff', '#00ff00', '#e6ff00', '#ffff00',
|
||||||
'#905000', '#9b0000', '#840067', '#9abe00', '#ffc900', '#510090', '#0000c9', '#009b00',
|
'#905000', '#9b0000', '#840067', '#9abe00', '#ffc900', '#510090', '#0000c9', '#009b00',
|
||||||
|
@ -156,69 +157,70 @@ openerp.base_graph.GraphView = openerp.base.Controller.extend({
|
||||||
view_chart = self.orientation == 'horizontal'? 'barH' : 'bar';
|
view_chart = self.orientation == 'horizontal'? 'barH' : 'bar';
|
||||||
}
|
}
|
||||||
|
|
||||||
for (var i in result){
|
_.each(results, function (result) {
|
||||||
if (self.group_field && (this.operator.length <= 1)){
|
if (self.group_field && (this.operator.length <= 1)){
|
||||||
newkey =result[i][self.group_field].split(' ').join('_');
|
newkey = result[self.group_field].split(' ').join('_');
|
||||||
}else{
|
} else {
|
||||||
newkey = "val";
|
newkey = "val";
|
||||||
}
|
}
|
||||||
|
|
||||||
if (jQuery.inArray(newkey, self.group_list) == -1){
|
if (jQuery.inArray(newkey, self.group_list) == -1){
|
||||||
self.group_list.push(newkey);
|
self.group_list.push(newkey);
|
||||||
if(this.operator.length > 1){
|
if(this.operator.length > 1){
|
||||||
var newkey_one = "val1";
|
newkey_one = "val1";
|
||||||
self.group_list.push(newkey_one);
|
self.group_list.push(newkey_one);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}, this);
|
||||||
|
|
||||||
for (var j in result){
|
_.each(results, function (result) {
|
||||||
var xystring = result[j][self.chart_info_fields];
|
var xystring = result[self.chart_info_fields];
|
||||||
if (self.group_field && (self.operator.length <= 1)){
|
if (self.group_field && (self.operator.length <= 1)){
|
||||||
newkey =result[j][self.group_field].split(' ').join('_');
|
newkey = result[self.group_field].split(' ').join('_');
|
||||||
}else{
|
}else{
|
||||||
newkey = "val";
|
newkey = "val";
|
||||||
}
|
}
|
||||||
if (xystr[xystring] == undefined){
|
if (xystr[xystring] == undefined){
|
||||||
xyname = {};
|
xyname = {};
|
||||||
xyname[self.chart_info_fields] = xystring;
|
xyname[self.chart_info_fields] = xystring;
|
||||||
for (var k in self.group_list){
|
_.each(self.group_list, function (group) {
|
||||||
xyname[self.group_list[k]] = 0.0001;
|
xyname[group] = 0.0001;
|
||||||
}
|
});
|
||||||
xyname[newkey] = result[j][self.operator_field];
|
xyname[newkey] = result[self.operator_field];
|
||||||
if (self.operator.length > 1){
|
if (self.operator.length > 1){
|
||||||
xyname[newkey_one] = result[j][self.operator_field_one];
|
xyname[newkey_one] = result[self.operator_field_one];
|
||||||
}
|
}
|
||||||
xystr[xystring] = xyname;
|
xystr[xystring] = xyname;
|
||||||
}
|
}
|
||||||
else{
|
else{
|
||||||
xyname = {};
|
xyname = {};
|
||||||
xyname = xystr[xystring];
|
xyname = xystr[xystring];
|
||||||
xyname[newkey] = result[j][self.operator_field];
|
xyname[newkey] = result[self.operator_field];
|
||||||
if (self.operator.length > 1){
|
if (self.operator.length > 1){
|
||||||
xyname[newkey_one] = result[j][self.operator_field_one];
|
xyname[newkey_one] = result[self.operator_field_one];
|
||||||
}
|
}
|
||||||
xystr[xystring] = xyname;
|
xystr[xystring] = xyname;
|
||||||
}
|
}
|
||||||
}
|
});
|
||||||
for (var label in xystr){
|
|
||||||
res.push(xystr[label]);
|
_.each(xystr, function (column_data) {
|
||||||
}
|
res.push(column_data);
|
||||||
|
});
|
||||||
|
|
||||||
//for legend color
|
//for legend color
|
||||||
var grp_color = [];
|
var grp_color = [];
|
||||||
for (var l in self.group_list){
|
_.each(self.group_list, function (group_legend, index) {
|
||||||
var legend = {};
|
var legend = {};
|
||||||
if (self.group_list[l] == "val"){
|
if (group_legend == "val"){
|
||||||
legend['text'] = self.fields[self.operator_field]['string']
|
legend['text'] = self.fields[self.operator_field]['string']
|
||||||
}else if(self.group_list[l] == "val1"){
|
}else if(group_legend == "val1"){
|
||||||
legend['text'] = self.fields[self.operator_field_one]['string']
|
legend['text'] = self.fields[self.operator_field_one]['string']
|
||||||
}else{
|
}else{
|
||||||
legend['text'] = self.group_list[l];
|
legend['text'] = group_legend;
|
||||||
}
|
}
|
||||||
legend['color'] = COLOR_PALETTE[l];
|
legend['color'] = COLOR_PALETTE[index];
|
||||||
grp_color.push(legend);
|
grp_color.push(legend);
|
||||||
}
|
});
|
||||||
|
|
||||||
//for axis's value and title
|
//for axis's value and title
|
||||||
var temp_ax = {};
|
var temp_ax = {};
|
||||||
|
|
|
@ -83,6 +83,91 @@ should *never* use trailing commas in Javascript object literals:
|
||||||
in object literals puts you at risks of using them in literal JSON strings
|
in object literals puts you at risks of using them in literal JSON strings
|
||||||
as well (though there are few reasons to write JSON by hand)
|
as well (though there are few reasons to write JSON by hand)
|
||||||
|
|
||||||
|
*Never* use ``for … in`` to iterate on arrays
|
||||||
|
*********************************************
|
||||||
|
|
||||||
|
:ref:`Iterating over an object with for…in is a bit tricky already
|
||||||
|
<for-in-iteration>`, it is far more complex than in Python (where it Just
|
||||||
|
Works™) due to the interaction of various Javascript features, but to iterate
|
||||||
|
on arrays it becomes downright deadly and errorneous: ``for…in`` really
|
||||||
|
iterates over an *object*'s *properties*.
|
||||||
|
|
||||||
|
With an array, this has the following consequences:
|
||||||
|
|
||||||
|
* It does not necessarily iterate in numerical order, nor does it iterate in
|
||||||
|
any kind of set order. The order is implementation-dependent and may vary
|
||||||
|
from one run to the next depending on a number of reasons and implementation
|
||||||
|
details.
|
||||||
|
* If properties are added to an array, to ``Array.prototype`` or to
|
||||||
|
``Object.prototype`` (the latter two should not happen in well-behaved
|
||||||
|
javascript code, but you never know...) those properties *will* be iterated
|
||||||
|
over by ``for…in``. While ``Object.hasOwnProperty`` will guard against
|
||||||
|
iterating prototype properties, they will not guard against properties set
|
||||||
|
on the array instance itself (as memoizers for instance).
|
||||||
|
|
||||||
|
Note that this includes setting negative keys on arrays.
|
||||||
|
|
||||||
|
For this reason, ``for…in`` should **never** be used on array objects. Instead,
|
||||||
|
you should use either a normal ``for`` or (even better, unless you have
|
||||||
|
profiled the code and found a hotspot) one of Underscore's array iteration
|
||||||
|
methods (`_.each`_, `_.map`_, `_.filter`_, etc...).
|
||||||
|
|
||||||
|
Underscore is guaranteed to be bundled and available in OpenERP Web scopes.
|
||||||
|
|
||||||
|
.. _for-in-iteration:
|
||||||
|
|
||||||
|
Use ``hasOwnProperty`` when iterating on an object with ``for … in``
|
||||||
|
********************************************************************
|
||||||
|
|
||||||
|
``for…in`` is Javascript's built-in facility for iterating over and object's
|
||||||
|
properties.
|
||||||
|
|
||||||
|
`It is also fairly tricky to use`_: it iterates over *all* non-builtin
|
||||||
|
properties of your objects [#]_, which includes methods of an object's class.
|
||||||
|
|
||||||
|
As a result, when iterating over an object with ``for…in`` the first line of
|
||||||
|
the body *should* generally be a call to `Object.hasOwnProperty`_. This call
|
||||||
|
will check whether the property was set directly on the object or comes from
|
||||||
|
the object's class:
|
||||||
|
|
||||||
|
.. code-block:: javascript
|
||||||
|
|
||||||
|
for(var key in ob) {
|
||||||
|
if (!ob.hasOwnProperty(key)) {
|
||||||
|
// comes from ob's class
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
// do stuff with key
|
||||||
|
}
|
||||||
|
|
||||||
|
Since properties can be added directly to e.g. ``Object.prototype`` (even
|
||||||
|
though it's usually considered bad style), you should not assume you ever know
|
||||||
|
which properties ``for…in`` is going to iterate over.
|
||||||
|
|
||||||
|
An alternative is to use Underscore's iteration methods, which generally work
|
||||||
|
over objects as well as arrays:
|
||||||
|
|
||||||
|
Instead of
|
||||||
|
|
||||||
|
.. code-block:: javascript
|
||||||
|
|
||||||
|
for (var key in ob) {
|
||||||
|
if (!ob.hasOwnProperty(key)) { continue; }
|
||||||
|
var value = ob[key];
|
||||||
|
// Do stuff with key and value
|
||||||
|
}
|
||||||
|
|
||||||
|
you could write:
|
||||||
|
|
||||||
|
.. code-block:: javascript
|
||||||
|
|
||||||
|
_.each(ob, function (value, key) {
|
||||||
|
// do stuff with key and value
|
||||||
|
});
|
||||||
|
|
||||||
|
and not worry about the details of the iteration: underscore should do the
|
||||||
|
right thing for you on its own [#]_.
|
||||||
|
|
||||||
Writing documentation
|
Writing documentation
|
||||||
+++++++++++++++++++++
|
+++++++++++++++++++++
|
||||||
|
|
||||||
|
@ -294,6 +379,30 @@ Roadmap
|
||||||
Release notes
|
Release notes
|
||||||
+++++++++++++
|
+++++++++++++
|
||||||
|
|
||||||
|
.. [#] More precisely, it iterates over all *enumerable* properties. It just
|
||||||
|
happens that built-in properties (such as ``String.indexOf`` or
|
||||||
|
``Object.toString``) are set to non-enumerable.
|
||||||
|
|
||||||
|
The enumerability of a property can be checked using
|
||||||
|
`Object.propertyIsEnumeable`_.
|
||||||
|
|
||||||
|
Before ECMAScript 5, it was not possible for user-defined properties
|
||||||
|
to be non-enumerable in a portable manner. ECMAScript 5 introduced
|
||||||
|
`Object.defineProperty`_ which lets user code create non-enumerable
|
||||||
|
properties (and more, read-only properties for instance, or implicit
|
||||||
|
getters and setters). However, support for these is not fully complete
|
||||||
|
at this point, and they are not being used in OpenERP Web code anyway.
|
||||||
|
|
||||||
|
.. [#] While using underscore is generally the preferred method (simpler,
|
||||||
|
more reliable and easier to write than a *correct* ``for…in``
|
||||||
|
iteration), it is also probably slower (due to the overhead of
|
||||||
|
calling a bunch of functions).
|
||||||
|
|
||||||
|
As a result, if you profile some code and find out that an underscore
|
||||||
|
method adds unacceptable overhead in a tight loop, you may want to
|
||||||
|
replace it with a ``for…in`` (or a regular ``for`` statement for
|
||||||
|
arrays).
|
||||||
|
|
||||||
.. [#] Because Python is the default domain, the ``py:`` markup prefix
|
.. [#] Because Python is the default domain, the ``py:`` markup prefix
|
||||||
is optional and should be left out.
|
is optional and should be left out.
|
||||||
|
|
||||||
|
@ -326,3 +435,17 @@ Release notes
|
||||||
http://code.google.com/p/jsdoc-toolkit/
|
http://code.google.com/p/jsdoc-toolkit/
|
||||||
.. _John Resig's Class implementation:
|
.. _John Resig's Class implementation:
|
||||||
http://ejohn.org/blog/simple-javascript-inheritance/
|
http://ejohn.org/blog/simple-javascript-inheritance/
|
||||||
|
.. _\_.each:
|
||||||
|
http://documentcloud.github.com/underscore/#each
|
||||||
|
.. _\_.map:
|
||||||
|
http://documentcloud.github.com/underscore/#map
|
||||||
|
.. _\_.filter:
|
||||||
|
http://documentcloud.github.com/underscore/#select
|
||||||
|
.. _It is also fairly tricky to use:
|
||||||
|
https://developer.mozilla.org/en/JavaScript/Reference/Statements/for...in#Description
|
||||||
|
.. _Object.propertyIsEnumeable:
|
||||||
|
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/propertyIsEnumerable
|
||||||
|
.. _Object.defineProperty:
|
||||||
|
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/defineProperty
|
||||||
|
.. _Object.hasOwnProperty:
|
||||||
|
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/hasOwnProperty
|
||||||
|
|
Loading…
Reference in New Issue