Yay, my first NEGATIVE response!!! GOOD! No joke, I'm actually pleased by this as it means ACTUAL Criticism on which I can base improvements and garner understanding, instead of the lame back-slapping "everything's good" I get most places.
I may finally be in the right place.
ShrineDesigns;1330669 wrote:If your library is library, then why does it not follow the conventions of a library? Most of all the functionality in elementals.js should be implemented as functions or as methods to a class.
You mean like these?
http://www.elementalsjs.com/_
Admittedly only about a third of the codebase, but still...
ShrineDesigns;1330669 wrote:Adding non-polyfills onto built-in objects adds "bloat" (and is bad practice in my opinion).
I assume you mean methods... I did beat myself up slightly about doing this at first, but really as I'm encouraging node-walking and pre-fetch over the realtime fetch nonsense like jQ and others do, and that it's STILL plenty fast (if not faster than what most 'frameworks' do) I'm not worried about it from a 'bloat' standpoint. Now, if I was attaching 60+k of methods to objects, THEN I'd be worried about it...
Though I really wish legacy IE did "Element.prototype" properly. I may still go back through and apply them as Element.prototype on modern browsers and _.extend on legacy.
I'm just not sold on the 'bad practice' part of extending objects -- my smalltalk and object pascal background screams at me that it's the entire POINT of having objects in the first place; extensibility, inheritance, etc, etc... Not leveraging that extensibility seems very inefficient to me, particularly when the other choice is increasing the amount of data passed on the 'stack' to functions. (assuming JS engines have something remotely resembling a stack).
I've heard a lot of 'don't do it' but not a lot of legitimate reasons as to 'why'. In fact I started to wonder if it was more 'placebo effect' of people saying it so it must be true even if there's no fact behind it. (kind of like the nonsense of jQuery being easier, HTML 5 being easier, Transitional being easier, OOCSS being easier, LESS being easier... and me quoting Inigo Montoya in response.)
Basically sounds like people taking a small "don't use it for this one thing" and turning it into a "don't use it ever". See nonsense like "don't use tables for layout" being magically turned into "don't use tables ever" nonsense, or the "strong and EM should be used instead of B and I when providing emphasis" being turned into the complete nonsense fiction of "Strong and EM replace B and I completely" -- to the point some people even go so far as to make wild nonsensical claims about B and I being deprecated.
Though if you can explain any pitfalls, I'd love to hear it! CONVINCE ME.
ShrineDesigns;1330669 wrote:The MDN has some very well written polyfills.
I thought so too -- in fact my Polyfills are based on their code... I just cut out the garbage that didn't seem to do anythign meaningful and seemed to just be left over from way too literal an interpretation of the spec -- a spec that was NOT written from the point of view of implementing those methods in JavaScript, but some other language like C.
MAYBE you could explain it to me? I asked on another forum and on the MDN mailing list and got some vague references to "Corner cases" that still didn't explain why the code did certain things that make no sense whatsoever in JS.
Let's use "Array.Every" as an example. The MDN polyfill is:
if (!Array.prototype.every)
{
Array.prototype.every = function(fun /*, thisArg */)
{
'use strict';
if (this === void 0 || this === null)
throw new TypeError();
var t = Object(this);
var len = t.length >>> 0;
if (typeof fun !== 'function')
throw new TypeError();
var thisArg = arguments.length >= 2 ? arguments[1] : void 0;
for (var i = 0; i < len; i++)
{
if (i in t && !fun.call(thisArg, t[i], i, t))
return false;
}
return true;
};
}
If 'this' is void or null, how the devil is an Array.prototype method even being called? The ONLY excuse would be forcing the issue with Function.prototype.call... which shoving the wrong type of variable to a object method SHOULD blow up in your face. If you're dumb enough to do it, you should get what's coming to you! Besides, how on earth is "this" null or void, while being type Array? How does that make ANY sense whatsoever? (In JavaScript... in C? Sure...) I mean... are people doing something really stupid like actually trying to call a .prototype method designed to process an instatiated Array directly?!?
Next is the part I really don't get -- the conversion to OBJECT... WHY? What possible reason does this serve? Just TRYING to make this harder to process? I mean, if you're worried about it being the wrong type to iterate, wouldn't converting it to Array make more sense? Again, the only reason to force typecasting would be if you use .call to force feed it an invalid variable, in which case why is plodding forward like nothing is wrong a good thing?!?
Or is this a really half-assed attempt at dealing with the fact that you might force an object or other variable into it (stupid) and you can convert array to object but can't convert object to array?
Next is the forcing of length to an integer with a shift-- Object.length or Array.length are integer unless you've forced something into it you NEVER should have forced into it (in which case it should be resolving to 0). That the spec is doing so via typecasting to uInt32 shows that this bit of code has NOTHING to do with implementing the function in JavaScript.
JS has a perfectly good system of determining if an argument exists or not, so why the devil are they using arguments.length? If the syntax is supposed to be:
arr.every(callback[, thisArg])
Why isn't the function declared:
Array.prototype.every = function(callback, thisArg)
Then simply checking if (typeof thisArg =='undefined')?!? Sorry, but my BS alarm is going off. Again though, that type of coding check makes perfect sense if you were implementing it in a language that didn't have optional parameter support built in. YES, it's EXACTLY how it's written in the spec, but the spec isn't written from the point of view of writing it in JavaScript because NEWS FLASH you don't write JavaScript engines for browsers in JavaScript!
Then the loop... oh the loop. They for/next through it, then check "if i in t" -- if it's been converted to an object isn't that the job of "for i in this"? I know some people again seem to think "never use for/in" because it fails on Arrays, but this is an OBJECT at this point. The only reason I can figure is that Object properties lose their order, but given that in legacy IE so do their numeric indexes, and that legacy IE is the only place you'd even HAVE this code running...
Though if they had just left it a bloody array (or typecast it as such), and that arrays auto-compact their numbered indexes, they wouldn't need that extra IF inside the loop, would they?
hence why my rewrite (verbose) is:
every : function(callback, thisArg) {
_.throwNonType(callback, 'function');
var len = this.length, k;
thisArg = typeof thisArg == 'undefined' ? void 0 : thisArg;
for (k=0; k < len; k++) if (!callback.call(thisArg, this[k], k, this)) return false;
return true;
}
That being from the object that is iterated and tested for against Array.prototype just like their IF statement does... since all the polyfills need that same check, it made more sense to put them into their own object and copy when needed to reduce the amount of code.
I MIGHT be convinced to add the 'this is null' throw back in (I do have the function for that check after all -- so just _.throwNull(this)) but other than that? in 99% of real world uses I find it doubtful anyone would notice the difference... and if that Object part is really necessary:
every : function(callback, thisArg) {
_.throwNull(this);
_.throwNonType(callback, 'function');
var t = Object(this), k;
thisArg = typeof thisArg == 'undefined' ? void 0 : thisArg;
for (k in t) if (!callback.call(thisArg, t[k], k, this)) return false;
return true;
}
Should be functionally identical to what they are doing -- actually no, it would be MORE functional since if you shove an object at it with .call, their implementation could skip over object properties since properties don't numerically collapse like Array indexes does.
Their polyfills -- particularly on Array -- while working and excellent at explaining what the function does, just seems to have a lot of stuff in it that has NOTHING to do with how JavaScript works -- on ANY browser or implementation I've ever heard of, and reeks of a bad port from C or C++ without taking into account the differences between the languages. Some of them even seem to BREAK compared to the actual browser spec versions -- as I just noted with the for/next integer loop over objects where it could skip over properties!
The "best" explanation anyone could give me was "It's being literal to the spec" -- I nice ideal but really if you're porting it to a language, shouldn't you maybe... take how that language works into account?!? The spec wasn't written TO JS implementation becuase again, JS engines aren't written in JS!
If I'm wrong, please EXPLAIN why I'm wrong, and WHY they are doing those things? I've been asking even on the MDN mailing list for a month and a half, and the answers I've gotten back have been little more than "because".
I would like an ACTUAL explanation. The lack of one is why I pushed on and just did it the way I did.
... contined on next post (too long)