Qooxdoo--garbage?

Prev: Page action
Next: Code posting guidelines (was: Qooxdoo--garbage?)

From: David Mark on 31 May 2010 22:42

---

Downloaded the qooxdoo SDK. Only 20MB; that's a bargain! Of course,
it is mostly the graphics used by the widgets. And if the low-level
code turns out to be the usual gobbledygook found in such frameworks,
the rest of it is just a waste of time and space.

Those wishing to save their own time should skip to the last
sentence. There's nothing new here. ;)

The source starts in \framework\source\class\qx. This folder contains
7 scripts. Bootstrap, Class, Interface, Mixin, Part, Theme and the
very oddly named __init__.js. My guess is that that last one is the
entry point.

/**
* This is the framework's top-level namespace or "package".
*
* It contains some fundamental classes, while the rest of the class
hierarchy
* is available in the corresponding sub packages. Please see the
descriptions
* below. The little **+** preceding a name offers more descriptive
text when
* you click on it.
*
* All packages are structured in the same way. Use either the
hyperlinks in
* the listing, or the tree structure to navigate to the corresponding
packages
* and their documentation. You can also use the __Search__ button to
find
* certain class and method names, and the __Legend__ button for
information
* about the symbols used throughout this reference. Use the toolbar
buttons to
* to the right to tailor the amount of information that is displayed
with each
* class, e.g. you can enable display of inherited or protected class
members.
*/

Well, not it isn't. It's just a comment. I sense Dojo-itis has
afflicted this project where everything is sliced and diced into as
many files as possible, so as to make it impossible to find anything.
Such granularity might be useful if (unlike Dojo) the many files
mirror the product's modularity.

Moving on to Bootstrap, assuming it will be the real entry point.

/**
* Create namespace
*/
if (!window.qx) {
window.qx = {};
}

Could really end the review right there. The first line of code
augments a host object for no reason other than ignorance. JFTR, that
should have read:-

var qx;

if (!qx) {
qx = {};
}

....or:-

var qx;

qx = qx || {};

The fact that they botched a global variable declaration doesn't bode
well for the remaining umpteen thousand lines of code.

__classToTypeMap :
{
"[object String]": "String",
"[object Array]": "Array",
"[object Object]": "Object",
"[object RegExp]": "RegExp",
"[object Number]": "Number",
"[object Boolean]": "Boolean",
"[object Date]": "Date",
"[object Function]": "Function",
"[object Error]": "Error"
},

There's a waste of space.

bind : function(func, self, varargs)
{
var fixedArgs = Array.prototype.slice.call(arguments, 2,
arguments.length);


Well, clearly they haven't yet mastered slice.


return function() {
var args = Array.prototype.slice.call(arguments, 0,
arguments.length);


Again.


return func.apply(self, fixedArgs.concat(args));
}
},

firstUp : function(str) {
return str.charAt(0).toUpperCase() + str.substr(1);
},

The substr method is non-standard.

http://mdn.beonex.com/en/Core_JavaScript_1.5_Reference/Global_Objects/String/substr#Summary

They should have used slice, but then they are pretty clumsy with that
one. It's interesting to note that they've been working on this for
*years*, but still haven't learned the basics of the language.
Recently Ajaxian described this thing as a "mature" framework, which I
assume referred to the amount of wasted time (and perhaps the huge
collection of graphics).

/**
* Get the internal class of the value. See
* http://thinkweb2.com/projects/prototype/instanceof-considered-harmful-or-how-to-write-a-robust-isarray/
* for details.

So they've read Kangax' blog, but apparently not closely. There is
never a good reason to write an isArray function. And it's impossible
anyway. Other than the userAgent property (which will likely pop up
soon), there's really no greater indicator of incompetence than an
isArray function.

Typically such things are used for "overloading" schemes that must
differentiate between Object and Array objects (and sometimes even
host objects. Don't use such designs as they are pinned on something
that is impossible.

*
* @param value {var} value to get the class for
* @return {String} the internal class of the value
*/
getClass : function(value)
{
var classString = Object.prototype.toString.call(value);
return (
qx.Bootstrap.__classToTypeMap[classString] ||
classString.slice(8, -1)
);
},


/**
* Whether the value is a string.
*
* @param value {var} Value to check.


And what is this value allowed to be?


* @return {Boolean} Whether the value is a string.
*/
isString : function(value)
{
// Added "value !== null" because IE throws an exception "Object
expected"
// by executing "value instanceof Array" if value is a DOM
element that
// doesn't exist. It seems that there is a internal different
between a
// JavaScript null and a null returned from calling DOM.
// e.q. by document.getElementById("ReturnedNull").


As noted in another thread, that's about as confused as it gets. And
apparently they are fine with passing host objects to these
functions. In other words, the whole thing is doomed from the start.


return (
value !== null && (
typeof value === "string" ||
qx.Bootstrap.getClass(value) == "String" ||
value instanceof String ||
(!!value && !!value.$$isString))
);
},

Disallow String objects (which shouldn't be used anyway) and you only
need the typeof check. The rest of the tacked on nonsense (and
accompanying confused comment) is simply flailing to try to make an
impossible design "work". God only knows what that last check is
supposed to be.

isArray : function(value)
{
// Added "value !== null" because IE throws an exception "Object
expected"
// by executing "value instanceof Array" if value is a DOM
element that
// doesn't exist. It seems that there is a internal different
between a
// JavaScript null and a null returned from calling DOM.
// e.q. by document.getElementById("ReturnedNull").


Deja vu.


return (
value !== null && (
value instanceof Array ||
(value && qx.data && qx.data.IListData &&
qx.Bootstrap.hasInterface(value.constructor, qx.data.IListData) ) ||
qx.Bootstrap.getClass(value) == "Array" ||
(!!value && !!value.$$isArray))
);
},


Odd rearrangement of the previous function's logic. They are trying
desperately to make this ill-advised design work cross-frame/window,
but are 0-2 at this point. They aren't even close.

Prediction: somebody will show up here and assert that these functions
pass their unit tests and demand to know where they fail. Make no
mistake that they can be demonstrated to fail, but that's actually
beside the point.

/**
* Whether the value is an object. Note that built-in types like
Window are
* not reported to be objects.


Built-in types like Window?


*
* @param value {var} Value to check.


Again, what is this value allowed to be? Is this really meant to
handle host objects? Considering the previous comment, it seems
unlikely they know what host objects are.


* @return {Boolean} Whether the value is an object.
*/
isObject : function(value) {
return (
value !== undefined &&
value !== null &&
qx.Bootstrap.getClass(value) == "Object"
);
},

/**
* Whether the value is a function.
*
* @param value {var} Value to check.
* @return {Boolean} Whether the value is a function.
*/
isFunction : function(value) {
return qx.Bootstrap.getClass(value) == "Function";
},


I have to wonder why they haven't "decorated" this one with the hacks
seen in the previous functions. Also have to wonder what on earth
they would use this for.

And if host objects (and RegExp objects) are excluded (as they
certainly should be), a typeof check would suffice. And, as with the
isString, a function that is nothing more than an typeof operation is
a wasted call (just use the typeof operator!) As the documentation is
simply "Whether the value is a function", there's no telling what they
are trying to accomplish with this. An attempt at an explanation
would certainly be welcome.

Moving on to Class:-

#require(qx.Interface)
#require(qx.Mixin)


There it is. What are the odds that those two scripts serve any
useful purpose on their own? Can you really build a qooxdoo project
without this Class script. If not, then there was no need to break
the them up.


// Normalize include to array
if (config.include && !(config.include instanceof Array)) {
config.include = [config.include];
}


Oops. Why didn't they call their "perfect" isArray function? Can't
help but think they might have noticed this if the code wasn't
scattered among multiple files.

So enough with the low-level language machinations. It's apparent
they are very new to JS (and have never heard of browser scripting).

bom/Selector.js:-

This class contains code based on the following work:

* Sizzle CSS Selector Engine - v1.0

http://sizzlejs.com/
http://groups.google.com/group/sizzlejs
http://github.com/jeresig/sizzle/tree

Snapshot from Oct 29 2009
commit b363fde6c7b55d43777b28eeb7ede5827e899ec9
Copyright:
(c) 2009, The Dojo Foundation

License:
MIT: http://www.opensource.org/licenses/mit-license.php


----------------------------------------------------------------------

Copyright (c) 2009 John Resig


The kiss of death. :(

bom/Viewport.js:-


This class contains code based on the following work:

* Yahoo! UI Library
http://developer.yahoo.com/yui
Version 2.2.0

Copyright:
(c) 2007, Yahoo! Inc.

License:
BSD: http://developer.yahoo.com/yui/license.txt


YUI is clueless to this day, but YUI circa 2007?

qx.Class.define("qx.bom.Viewport",
{
statics :
{
/**
* Returns the current width of the viewport (excluding a
eventually visible scrollbar).
*
* <code>clientWidth</code> is the inner width of an element in
pixels. It includes padding
* but not the vertical scrollbar (if present, if rendered),
border or margin.
*
* The property <code>innerWidth</code> is not useable as defined
by the standard as it includes the scrollbars
* which is not the indented behavior of this method. We can
decrement the size by the scrollbar
* size but there are easier possibilities to work around this.
*
* Safari 2 and 3 beta (3.0.2) do not correctly implement
<code>clientWidth</code> on documentElement/body,
* but <code>innerWidth</code> works there. Interesting is that
webkit do not correctly implement
* <code>innerWidth</code>, too. It calculates the size excluding
the scroll bars and this
* differs from the behavior of all other browsers - but this is
exactly what we want to have
* in this case.
*
* Opera less then 9.50 only works well using
<code>body.clientWidth</code>.
*
* Verified to correctly work with:
*
* * Mozilla Firefox 2.0.0.4
* * Opera 9.2.1
* * Safari 3.0 beta (3.0.2)
* * Internet Explorer 7.0


LOL. What about Opera 9.50?


*
* @signature function(win)
* @param win {Window?window} The window to query
* @return {Integer} The width of the viewable area of the page
(excludes scrollbars).
*/
getWidth : qx.core.Variant.select("qx.client",
{
"opera" : function(win) {
if (qx.bom.client.Engine.VERSION < 9.5) {
return (win||window).document.body.clientWidth;
}
else
{
var doc = (win||window).document;
return qx.bom.Document.isStandardMode(win) ?
doc.documentElement.clientWidth : doc.body.clientWidth;
}
},

"webkit" : function(win) {
if (qx.bom.client.Engine.VERSION < 523.15) { // Version <
3.0.4
return (win||window).innerWidth;
}
else
{
var doc = (win||window).document;
return qx.bom.Document.isStandardMode(win) ?
doc.documentElement.clientWidth : doc.body.clientWidth;
}
},

"default" : function(win)
{
var doc = (win||window).document;
return qx.bom.Document.isStandardMode(win) ?
doc.documentElement.clientWidth : doc.body.clientWidth;
}
}),


All pinned on dated observations (of four browsers, one of which was
in Beta at the time).

http://www.cinsoft.net/viewport.asp

/**
* Returns the scroll position of the viewport
*
* All clients except MSHTML supports the non-standard property
<code>pageXOffset</code>.


All clients?! It's admittedly a non-standard property.


* As this is easier to evaluate we prefer this property over
<code>scrollLeft</code>.
*
* For MSHTML the access method differs between standard and
quirks mode;
* as this can differ from document to document this test must be
made on
* each query.
*
* Verified to correctly work with:
*
* * Mozilla Firefox 2.0.0.4
* * Opera 9.2.1
* * Safari 3.0 beta (3.0.2)
* * Internet Explorer 7.0


Does that lot represent all clients?


*
* @signature function(win)
* @param win {Window?window} The window to query
* @return {Integer} Scroll position from left edge, always a
positive integer
*/
getScrollLeft : qx.core.Variant.select("qx.client",
{
"mshtml" : function(win)
{
var doc = (win||window).document;
return doc.documentElement.scrollLeft || doc.body.scrollLeft;
},

"default" : function(win) {
return (win||window).pageXOffset;
}
}),


Let's see about this browser sniffing.

bom/client/Browser.js

This class contains code from:

Copyright:
2009 Deutsche Telekom AG, Germany, http://telekom.com

I suppose a phone company is as good a source as any for browser
scripting (can't do much worse than jQuery and YUI).


defer : qx.core.Variant.select("qx.client",
{
"webkit" : function(statics)
{
// Safari should be the last one to check, because some other
Webkit-based browsers
// use this identifier together with their own one.
// "Version" is used in Safari 4 to define the Safari version.
After "Safari" they place the
// Webkit version instead. Silly.


No, what is silly is parsing the UA string in the first place. The
very nature of these comments should make that obvious.

http://www.jibbering.com/faq/notes/detect-browser/


// Palm Pre uses both Safari (contains Webkit version) and
"Version" contains the "Pre" version. But
// as "Version" is not Safari here, we better detect this as the
Pre-Browser version. So place
// "Pre" in front of both "Version" and "Safari".


Well, glad we got all of that sorted out. :) For today (or whenever
this was released) anyway. Clearly this code, on which the rest of
this framework hangs, will need to be rewritten constantly.


statics.__detect("AdobeAIR|Titanium|Fluid|Chrome|Android|
Epiphany|Konqueror|iCab|OmniWeb|Maxthon|Pre|Mobile Safari|Safari");
},

"gecko" : function(statics)
{
// Better security by keeping Firefox the last one to match


Better security?


statics.__detect("prism|Fennec|Camino|Kmeleon|Galeon|Netscape|
SeaMonkey|Firefox");
},

"mshtml" : function(statics)
{
// No idea what other browsers based on IE's engine


Aha! :)


statics.__detect("IEMobile|Maxthon|MSIE");


And no idea of the hundreds (if not thousands) of non-IE agents that
have "MSIE" in their UA string.


},

"opera" : function(statics)
{
// Keep "Opera" the last one to correctly prefer/match the
mobile clients
statics.__detect("Opera Mini|Opera Mobi|Opera");
}
})
});


bom/client/Multimedia.js

/**
* Internal initialize helper
*
* @return {void}
* @signature function()
*/
__init : qx.core.Variant.select("qx.client",
{
"mshtml" : function()
{
var control = window.ActiveXObject;
if (!control) {
return;
}


Basically, they rely on those ridiculous UA inferences (borrowed from
a phone company) for everything. In 2010. And such code would have
been unworkable (and laughable) in 2001. Forget it.

From: Garrett Smith on 1 Jun 2010 02:00

---

On 5/31/2010 7:42 PM, David Mark wrote:
> Downloaded the qooxdoo SDK. Only 20MB; that's a bargain! Of course,
> it is mostly the graphics used by the widgets. And if the low-level
> code turns out to be the usual gobbledygook found in such frameworks,
> the rest of it is just a waste of time and space.
>
> Those wishing to save their own time should skip to the last
> sentence. There's nothing new here. ;)
>

I see some loose inferences that resemble those in Mootools. Basically,
it is the idea of using something other than navigator.userAgent to
detect a browser, where that information can later be used to address
yet another situation.

Incidentally, HTML 5 draft is standardizing the navigator object.

| userAgent
|
| Must return the string used for the value of the "User-Agent" header
| in HTTP requests, or the empty string if no such header is ever sent.
|

This is something that HTML 5 got right. Trying to nail down specifics
of formatting would have been foolish.

navigator.platform is also specified:

| platform
|
| Must return either the empty string or a string representing the
| platform on which the browser is executing, e.g. "MacIntel", "Win32",
| "FreeBSD i386", "WebTV OS"

> The source starts in \framework\source\class\qx. This folder contains
> 7 scripts. Bootstrap, Class, Interface, Mixin, Part, Theme and the
> very oddly named __init__.js. My guess is that that last one is the
> entry point.
>
> /**
> * This is the framework's top-level namespace or "package".
> *
> * It contains some fundamental classes, while the rest of the class
> hierarchy
> * is available in the corresponding sub packages. Please see the
> descriptions

The code is wrapping. It is not executable as transmitted and is harder
to read than it would be, had it been formatted to < 72 character width.

http://jibbering.com/faq/#postCode
http://jibbering.com/faq/notes/posting/#ps1Lw
http://jibbering.com/faq/notes/code-guidelines/#formatting

Comments or corrections to those documents are welcome -- I'll try to
fix them if they can be pointed out.

Garrett

From: Ry Nohryb on 1 Jun 2010 05:03

---

On Jun 1, 4:42 am, David Mark <dmark.cins...(a)gmail.com> wrote:
>
> /**
>  * Create namespace
>  */
> if (!window.qx) {
>   window.qx = {};
>
> }
>
> Could really end the review right there.  The first line of code
> augments a host object for no reason other than ignorance.

"window" is not a host object. It's an alias of the global object:

(function () {return this})().k= 27;
window.k
--> 27

That Microsoft's Internet Explorers got -even- "window" botched -as
well-, does not prove anything wrt this.

> JFTR, (...)
--
Jorge.

From: David Mark on 1 Jun 2010 10:09

---

On Jun 1, 5:03 am, Ry Nohryb <jo...(a)jorgechamorro.com> wrote:
> On Jun 1, 4:42 am, David Mark <dmark.cins...(a)gmail.com> wrote:
>
>
>
> > /**
> >  * Create namespace
> >  */
> > if (!window.qx) {
> >   window.qx = {};
>
> > }
>
> > Could really end the review right there.  The first line of code
> > augments a host object for no reason other than ignorance.
>
> "window" is not a host object.

It most assuredly is a host object, provided by browsers.


> It's an alias of the global object:

There's no proof of that whatsoever. Nor is there any standard that
dictates such.

>
> (function () {return this})().k= 27;
> window.k
> --> 27

And there's no proof, right there. :) And regardless, would you
seriously consider creating a global property by setting a property of
window?

>
> That Microsoft's Internet Explorers got -even- "window" botched -as
> well-, does not prove anything wrt this.

There's no way to botch a host object as there are no rules for host
objects.

http://www.cinsoft.net/host.html

From: Gabriel Gilini on 1 Jun 2010 10:33

---

On Jun 1, 6:03 am, Ry Nohryb <jo...(a)jorgechamorro.com> wrote:
> On Jun 1, 4:42 am, David Mark <dmark.cins...(a)gmail.com> wrote:
>
>
>
> > /**
> >  * Create namespace
> >  */
> > if (!window.qx) {
> >   window.qx = {};
>
> > }
>
> > Could really end the review right there.  The first line of code
> > augments a host object for no reason other than ignorance.
>
> "window" is not a host object. It's an alias of the global object:
No it isn't. It is a property of the global object.

 |  Next  |  Last
Pages: 1 2
Prev: Page action
Next: Code posting guidelines (was: Qooxdoo--garbage?)