diff --git a/addons/base_graph/static/src/js/graph.js b/addons/base_graph/static/src/js/graph.js index bd54a68a117..2aa290e61f2 100644 --- a/addons/base_graph/static/src/js/graph.js +++ b/addons/base_graph/static/src/js/graph.js @@ -39,17 +39,18 @@ openerp.base_graph.GraphView = openerp.base.Controller.extend({ this.group_field = ''; this.orientation = this.fields_view.arch.attrs.orientation || ''; - for(var fld in this.fields_view.arch.children) { - if (this.fields_view.arch.children[fld].attrs.operator) { - this.operator.push(this.fields_view.arch.children[fld].attrs.name); + _.each(this.fields_view.arch.children, function (field) { + if (field.attrs.operator) { + this.operator.push(field.attrs.name); } - else if (this.fields_view.arch.children[fld].attrs.group) { - this.group_field = this.fields_view.arch.children[fld].attrs.name; + else if (field.attrs.group) { + this.group_field = field.attrs.name; } 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]; if(this.operator.length > 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.opration_fld = {}; - if (result.length){ - for(var res in result) { - for(var fld in result[res]) { - if (typeof result[res][fld] == 'object') { - result[res][fld] = result[res][fld][result[res][fld].length - 1]; + if (results.length){ + _.each(results, function (result) { + _.each(result, function (field_value, field_name) { + if (typeof field_value == 'object') { + result[field_name] = field_value[field_value.length - 1]; } - if (typeof result[res][fld] == 'string'){ - if (this.all_fields[fld]['selection']){ - for (var i in this.all_fields[fld]['selection']){ - if(result[res][fld] == this.all_fields[fld]['selection'][i][0]){ - result[res][fld] = this.all_fields[fld]['selection'][i]; - } + if (typeof field_value == 'string'){ + var choices = this.all_fields[field_name]['selection']; + _.each(choices, function (choice) { + if(field_value == choice[0]){ + result[field_name] = choice; } - } + }); } - } - } + }, this); + }, this); - for (var j in result){ - var gen_key = result[j][this.chart_info_fields]+"_"+result[j][this.group_field]; + _.each(results, function (result) { + var gen_key = result[this.chart_info_fields]+"_"+result[this.group_field]; if (this.opration_fld[gen_key] == undefined){ 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){ - 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){ - 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; - }else{ + } else { 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){ - 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); + var graph_data = []; - for (var op in this.opration_fld){ - graph_data.push(this.opration_fld[op]); - } + _.each(this.opration_fld, function (column_data) { + graph_data.push(column_data); + }); if(this.chart == 'bar') { 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 view_chart = ''; var xystr = {}; var xyname = {}; var res = []; 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', '#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'; } - for (var i in result){ + _.each(results, function (result) { if (self.group_field && (this.operator.length <= 1)){ - newkey =result[i][self.group_field].split(' ').join('_'); - }else{ + newkey = result[self.group_field].split(' ').join('_'); + } else { newkey = "val"; } if (jQuery.inArray(newkey, self.group_list) == -1){ self.group_list.push(newkey); if(this.operator.length > 1){ - var newkey_one = "val1"; + newkey_one = "val1"; self.group_list.push(newkey_one); } } - } + }, this); - for (var j in result){ - var xystring = result[j][self.chart_info_fields]; + _.each(results, function (result) { + var xystring = result[self.chart_info_fields]; if (self.group_field && (self.operator.length <= 1)){ - newkey =result[j][self.group_field].split(' ').join('_'); + newkey = result[self.group_field].split(' ').join('_'); }else{ newkey = "val"; } if (xystr[xystring] == undefined){ xyname = {}; xyname[self.chart_info_fields] = xystring; - for (var k in self.group_list){ - xyname[self.group_list[k]] = 0.0001; - } - xyname[newkey] = result[j][self.operator_field]; + _.each(self.group_list, function (group) { + xyname[group] = 0.0001; + }); + xyname[newkey] = result[self.operator_field]; 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; } else{ xyname = {}; xyname = xystr[xystring]; - xyname[newkey] = result[j][self.operator_field]; + xyname[newkey] = result[self.operator_field]; 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; } - } - for (var label in xystr){ - res.push(xystr[label]); - } + }); + + _.each(xystr, function (column_data) { + res.push(column_data); + }); //for legend color var grp_color = []; - for (var l in self.group_list){ + _.each(self.group_list, function (group_legend, index) { var legend = {}; - if (self.group_list[l] == "val"){ + if (group_legend == "val"){ 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'] }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); - } + }); //for axis's value and title var temp_ax = {}; diff --git a/doc/source/project.rst b/doc/source/project.rst index 37a16159448..aa465aa3f9f 100644 --- a/doc/source/project.rst +++ b/doc/source/project.rst @@ -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 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 +`, 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 +++++++++++++++++++++ @@ -294,6 +379,30 @@ Roadmap 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 is optional and should be left out. @@ -326,3 +435,17 @@ Release notes http://code.google.com/p/jsdoc-toolkit/ .. _John Resig's Class implementation: 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