Mailing-List: contact dev-help@shindig.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@shindig.apache.org
Content-Type: multipart/alternative;
	boundary="===============8245652268726026755=="
MIME-Version: 1.0
Subject: Re: Review Request: Changing equality checks against undefined to
 typeof
 checks against 'undefined'
From: "Stanton Sievers" <sieverssj@gmail.com>
To: "shindig" <dev@shindig.apache.org>, "Dan Dumont" <ddumont@us.ibm.com>,
	"Paul Lindner" <lindner@inuus.com>, "Stanton Sievers" <sieverssj@gmail.com>
Date: Fri, 06 Jan 2012 22:54:18 -0000
Message-ID: <20120106225418.3460.31278@reviews.apache.org>
In-Reply-To: <20120103084527.3461.92296@reviews.apache.org>
References: <20120103084527.3461.92296@reviews.apache.org>

--===============8245652268726026755==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


> On 2012-01-03 08:45:27, Paul Lindner wrote:
> > I'm okay with the patch as-is, but would prefer something like this whi=
ch is a modified version of the closure library goog.isDef
> > =

> > =

> > goog.isDef =3D function(val) {
> >   var undefined;
> >   return val !=3D=3D undefined;
> > };
> >
> =

> Dan Dumont wrote:
>     Paul, is google's deployment running with advanced optimizations turn=
ed 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 u=
p for trying that... although, if we were going to introduce goog.isDef I w=
ould 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 "und=
efined" keyword, i.e., "foo !=3D undefined".  I've replaced such instances =
with "typeof foo !=3D 'undefined'" instead.  This eliminates possible error=
s that can arise if the "undefined" keyword is reassigned.  =

> =

> A little background regarding the motivation for this patch.... As of ECM=
AScript 3 and before ECMAScript 5, undefined is a keyword but is not restri=
cted, thus, it's value can be re-assigned.  This is generally caused by a p=
rogramming error, such as assigning values to object attributes without fir=
st checking that the object attribute exists - making them undefined.  When=
 these types of errors occur they are difficult to track down and debug.  T=
his patch is one way to help reduce possible errors caused by undefined bei=
ng re-assigned.
> =

> To find instances to replace, I searched for this regular expression in t=
he code base using Eclipse search within *.js files: =3D[ ]*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/e=
xamples/commoncontainer/cconviews.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/container.util/util.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/core.io/io.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/core.json/json-flatten.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/i18n/datetimeparse.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/open-views/viewenhancements-container.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/shindig.container/shindig-container.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascr=
ipt/features/shindig.uri/uri.js 1225624 =

>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascr=
ipt/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
> =

>


--===============8245652268726026755==--