Thrift
  1. Thrift
  2. THRIFT-1545

Generated javascript code uses "for in" for looping over arrays

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.8
    • Fix Version/s: 0.10.0
    • Component/s: JavaScript - Compiler
    • Labels:
      None

      Description

      For lists, generated javascript is like :
      >>

      GetRepairersResponse.prototype.write = function(output){
      output.writeStructBegin('GetRepairersResponse')
      if (null != this.repairers) {
      output.writeFieldBegin('repairers', Thrift.Type.LIST, 1)
      {
      output.writeListBegin(Thrift.Type.STRUCT, this.repairers.length)
      {
      for(var iter62 in this.repairers)

      { iter62=this.repairers[iter62] iter62.write(output) }

      }
      output.writeListEnd()
      }
      output.writeFieldEnd()
      }
      output.writeFieldStop()
      output.writeStructEnd()
      return
      }
      >>

      The use of "for in" generates problems when properties or functions are added to the Array object (for instance in the sencha library, that adds the "indexOf", "remove" and "contains" methods), because these properties will be included in the for loop.

      As said in https://developer.mozilla.org/en/JavaScript/Guide/Predefined_Core_Objects,: "Since JavaScript elements are saved as standard object properties, it is not advisable to iterate through JavaScript arrays using for...in loops because normal elements and all enumerable properties will be listed."

      It would be much safer if the generated code used standard for loops to like :
      >>
      var colors = ['red', 'green', 'blue'];
      for (var i = 0; i < colors.length; i++)

      { console.log(colors[i]); }


      >>

        Issue Links

          Activity

          Hide
          Brian Peiris added a comment -

          This was resolved with THRIFT-1033 which added "hasOwnProperty" checks in the "for in" loops.

          Show
          Brian Peiris added a comment - This was resolved with THRIFT-1033 which added "hasOwnProperty" checks in the "for in" loops.

            People

            • Assignee:
              Unassigned
              Reporter:
              Larregoity
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development