hebcal / hebcal-js

⛔️ DEPRECATED - a perpetual Jewish Calendar (JavaScript)
GNU General Public License v3.0
123 stars 40 forks source link

Why h[push]() instead of h.push()? #11

Closed mjradwin closed 10 years ago

mjradwin commented 10 years ago

@Scimonster, a naive JavaScript question for you.

In holidays.js, why do you do this:

    var push = 'push';
    h[push](new Event(
            RH,
            ['Rosh Hashana 1', 0, 'ראש השנה א\''],
            LIGHT_CANDLES_TZEIS
    ));

Instead of

    h.push(new Event(
            RH,
            ['Rosh Hashana 1', 0, 'ראש השנה א\''],
            LIGHT_CANDLES_TZEIS
    ));
Scimonster commented 10 years ago

To save bytes in the minified client version. It doesn't affect readability much, or usability. Syntax highlighting gets messed up a little, but that's minor.

When it gets minified, the variable names get shortened, typically to a single character. So let's see: h.push is 6 bytes. h[p] is only 4. This isn't much savings on its own, but when you have it 30 times, it can make a difference. I cut nearly 10KB from the minified version like that.

mjradwin commented 10 years ago

Got it. Thanks for the explanation! I understand the rationale. I'm relatively new to JS development, so it did indeed affect readability for me.

OK, another JS style question. Why give an actual name on both the LHS and RHS of this assignment:

HDate[prototype].getMonthName = function getMonthName(o) {
    return c.LANGUAGE(c.monthNames[+this.isLeapYear()][this[getMonth]()], o);
};

Instead of omitting on the RHS?

HDate[prototype].getMonthName = function(o) {
    return c.LANGUAGE(c.monthNames[+this.isLeapYear()][this[getMonth]()], o);
};
Scimonster commented 10 years ago

The LHS is an identifier; the RHS is the actual function name. That's the technical difference.

I read in a few places that it might be preferable, but i'm kinda doubting that now. Most modern systems are smart enough to figure out debugging even without a function name. It also caused a few unintended errors, such as the one that i fixed in 222d34d56943cd199584f72e24bac552df5738ca.

There are a couple cases where it is actually important to give the function a name, for convenience, such as in Hebcal.prototype.find(). There i actually use properties of the function, namely on line 219 there.

Scimonster commented 10 years ago

Notice also that i used HDate[prototype]. There, the savings are much more noticeable. n.prototype is 11 bytes; n[p] is only 4.

dsadinoff commented 10 years ago

Scimonster, no offense, but the h[push] syntax seems like a premature optimization to me, something to be avoided.

I advise you to let the minifier do it's job, and do your best to express the program in the clearest language possible. Is there a particular reason you're focused on optimizing the size of the minified javascript? Recall that nearly all javascript is cached after the first load, and moving via HTTP with a gzip content-encoding...

If there's an optimization that you'd like to see in the minifier, then I advise you to upgrade the minifier to one that can perform better, don't warp the source-code.

Scimonster commented 10 years ago

I suppose i'm just nitpicking at the filesize. I'm not exactly sure why, but i do prefer to write shorter code. It might not be totally necessary, but it is better. Of course, this isn't like the time i added jQuery (50-something KB before gzipping) to remove around 5KB from my code. :P

I intentionally made the non-minified source as readable as possible with the optimizations. It's not as if i named the alias of 'push' pop, or even shove. :P

No minifier is smart enough to identify where and how certain code can be compressed like this, AFAIK. I'm also not good enough at algorithms to even attempt something like that. :)