shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stanton Sievers" <siever...@gmail.com>
Subject Re: Review Request: Changing equality checks against undefined to typeof checks against 'undefined'
Date Fri, 06 Jan 2012 22:54:18 GMT


> On 2012-01-03 08:45:27, Paul Lindner wrote:
> > I'm okay with the patch as-is, but would prefer something like this which is a modified
version of the closure library goog.isDef
> > 
> > 
> > goog.isDef = function(val) {
> >   var undefined;
> >   return val !== undefined;
> > };
> >
> 
> Dan Dumont wrote:
>     Paul, is google's deployment running with advanced optimizations turned on?

Paul, where would such a definition live such that it is accessible across all of the JavaScript
files?  Any idea how we could hook that up?  I'd be up for trying that... although, if we
were going to introduce goog.isDef I would like to replace all typeof instances with that,
which is going to be a much bigger change.


- Stanton


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3332/#review4174
-----------------------------------------------------------


On 2011-12-29 20:05:42, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3332/
> -----------------------------------------------------------
> 
> (Updated 2011-12-29 20:05:42)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> The purpose of this patch is to remove equality checking against the "undefined" keyword,
i.e., "foo != undefined".  I've replaced such instances with "typeof foo != 'undefined'" instead.
 This eliminates possible errors that can arise if the "undefined" keyword is reassigned.
 
> 
> A little background regarding the motivation for this patch.... As of ECMAScript 3 and
before ECMAScript 5, undefined is a keyword but is not restricted, thus, it's value can be
re-assigned.  This is generally caused by a programming error, such as assigning values to
object attributes without first checking that the object attribute exists - making them undefined.
 When these types of errors occur they are difficult to track down and debug.  This patch
is one way to help reduce possible errors caused by undefined being re-assigned.
> 
> To find instances to replace, I searched for this regular expression in the code base
using Eclipse search within *.js files: =[ ]*undefined[ ]*
> 
> I ignored certain files, such as external libs (CodeMirror, etc) and most tests.
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/cconviews.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.json/json-flatten.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/i18n/datetimeparse.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shindig.container/shindig-container.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
1225624 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/osapi/batchtest.js
1225624 
> 
> Diff: https://reviews.apache.org/r/3332/diff
> 
> 
> Testing
> -------
> 
> Built everything.  Ensured that the common container sample gadgets still render.
> 
> 
> Thanks,
> 
> Stanton
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message