Opened 12 years ago

Closed 12 years ago

#4495 closed defect (fixed)

Use of assignment equals instead of comparison ==

Reported by: guest Owned by: James Burke
Priority: high Milestone: 1.0
Component: General Version: 0.9
Keywords: Cc:
Blocked By: Blocking:

Description

Loaded dojo.js.uncompressed.js in Aptana IDE and discovered that on line 59 = was used in the while loop rather than the comparison operator ==. Here is the code snippet for search purposes:

while(tn=cn[i++]){

if(!console[tn]){

console[tn] = function(){};

}

}

Change History (7)

comment:1 Changed 12 years ago by dante

it's in dojo/_base/_loader/bootstrap.js is is harmless. cn is an array defined immediately, and all supported browsers will exit the while when tn = undefined, which cn[i++] will always reach (cn.length)... its console.* debugging stuff.

 var cn = [
                "assert", "count", "debug", "dir", "dirxml", "error", "group",
                "groupEnd", "info", "log", "profile", "profileEnd", "time",
                "timeEnd", "trace", "warn"
        ];
        var i=0, tn;
        while(tn=cn[i++]){
                if(!console[tn]){
                        console[tn] = function(){};
                }
        }

comment:2 Changed 12 years ago by James Burke

Milestone: 1.0
Resolution: invalid
Status: newclosed

This is actually working as designed. This is a valid JS construct.

I suggest opening a bug with Aptana to find out why this was reported as an error.

comment:3 Changed 12 years ago by Adam Peller

It's considered 'bad style' by some. Crockford's jslint, and now Rhino strict mode, require an extra set of parenthesis around the assignment so that it stands out. Although I know some of the jslint stuff is controversial, I think this is good practice. It looks different than a comparison and stands out to the programmer, who might read it wrong.

comment:4 Changed 12 years ago by James Burke

Resolution: invalid
Status: closedreopened

peller: I'm OK wrapping the assignment in another set of parentheses if that solves the problem (will it?). I suppose Aptana is using jslint to generate these messages. Although, it seems to bring up a larger question that I think you have asked before: should the Dojo toolkit pass all the jslint checks? In the past, it seems like there was feedback from other contributors who felt like it was not appropriate.

But I'm fine with doing small things like this if it reduces messages when using dojo in an IDE that uses jslint.

comment:5 Changed 12 years ago by James Burke

Owner: changed from anonymous to James Burke
Status: reopenednew

comment:6 Changed 12 years ago by Adam Peller

Yup. That's a discussion we should eventually have regarding #3970, where I'd like to put such rules on the pre-commit hooks, if everyone is game.... something I plan to raise *after* 1.0 :-) I haven't looked too closely at the new Rhino feature. Not sure it's even released yet.

comment:7 Changed 12 years ago by James Burke

Resolution: fixed
Status: newclosed

(In [10713]) Fixes #4495. The extra set () should avoid the jslint error.

Note: See TracTickets for help on using tickets.