1409963 - Comm-Central: Introduce $() as a global shorthand helper function for document.getElementById()

Status: RESOLVED WONTFIX

(MailNews Core :: Backend, enhancement)

Description

[Thomas D. (:thomas8)]

User story: When coding up my patch attachment 8919861 [details] [diff] [review] in bug 663695, I got tired of writing document.getElementById(...) for the umpteenth time, and even if you need the element just once, it'll never fit on the same line with what you really want to get at. So I decided to make coder's life easier and the code neater by introducing the following shorthand helper function:

> var $ = function(aId) {
>  return document.getElementById(aId);
> }

Of course Jörg (aka "the Sheriff") immediately spotted the divergent new creature and demanded an explanation for this "funky thing". Sure.

I'm hereby proposing to make this helper function available globally in our codebase (comm-central).

Here's 7 reasons why document.getElementById() sucks and $() rocks:

1) document.getElementById(): An omni-present function

document.getElementById() is a very frequently used function for accessing elements across our codebase; more precisely, 33,231 occurences in comm-central.

2) Near-zero informational value

The informational/descriptive value of that function call is very low:
document - near zero informational value, that's the typical execution context, unless explicitly stated otherwise
getElementById - low informational value, because:
At the end of the day, the net result is just "THE ELEMENT with ID xyz", and assuming IDs have telling names, the core informational value is in the ID itself, not in "the element". Compare:

> document.getElementById("attachmentBucket") vs.
> $("attachmentBucket") vs.
> attachmentBucket

The informational/descriptive value is practically the same.
Imagine every time we refer to Jörg, we'd write like this:
|The Thunderbird coder from Germany by the name "Jörg"|, whilst "Jörg" is actually a well-known entity in our context so we can just say "Jörg" and still everyone will know exactly who we are talking about. The informational value is in the name, not in the accessor.

3) Bulky for production; waste of developer time and focus

Contrary to the everyday, trivial nature of this function and it's low informational value, it is incredibly BULKY and pretty hard to type during coding, due to its length and complex camel casing; even with auto-completion, you have to type almost all the way up to "document.getElementB..." before you get an unambiguous match for completion. This simple thing will slow you down and waste time and focus every time you use it, and it's very easy to misspell. I wonder how many man-hours have been lost for just typing this worthless function name. Compare:
> document.getElementById (23 chars, incl. dot, complex camel casing, and "Id" is just so counter-intuitive)
vs.
> $ (1 char - no complexity, no-brainer)
Iow, this function name is 2200% longer than it needs to be from a minimalist pov.

4) Code-base bloat

The lengthy function name is actually bloating the size of our codebase:
33,231 x 23 Bytes = 764,313 Bytes = 746 KB = 0.73 MB
A typical TB installation has 141 MB according to installer, so that's
0.73 MB / 141 MB = 0.52% of our total codebase size.
It might appear small in absolute numbers, but for a single function name which is not in the class of even more ubiquitous things like 'let', imo that's still quite significant, more so as you only use it once in every micro-context.

5) Bulky code, less readable, less density/compactness

document.getElementById() makes code significantly less readable and more bulky, as it diverts attention to the obvious, whilst distracting from the essence (the actual element ID, see above). It also forces unnecessary line wraps if used to access some deeper things.
Compare:

> let filename = document.getElementById("attachmentBucket").getItemAtIndex[0]
>                                                           .attachment.name
vs.
> let filename = $("attachmentBucket").getItemAtIndex[0].attachment.name

6) Coding style: unnecessary variables, memory footprint, extra lines and complexity in code

Due to 5), let's be honest, we often resort to creating variables which just hold the element, even when there's no need for such variables. So we'd probably rewrite 5) like this, just to make the code more readable:

> let attachmentBucket = document.getElementById("attachmentBucket");
> let filename = attachmentBucket.getItemAtIndex[0].attachment.name;

That's 1 extra line and one extra variable where sometimes no extra line and no extra variable might suffice.
Bingo! Compare the sweet simplicity and terseness of this:

> let filename = $("attachmentBucket").getItemAtIndex[0].attachment.name

It's much shorter, and much more readable, and probably using less memory as we don't create another intermediate variable.
On the downside, it's an extra function call but I think computing power usually isn't a problem these days and this isn't heavy.

7) Addressing maintenance concerns for $()

$ is known from jQuery where it has a slightly different meaning, because it'll return the jQuery Object representation of the element, not the actual element itself.
Well, we're not using jQuery, so imo it doesn't matter. Being more productive and having more readable, more compact code matters.
We're never going to use $ as a function name for anything else.
If we're ever going to use jQuery, no problem:
Since $(...) is a unique pattern which cannot possibly be used for anything else, we can always run a simple pattern search or even a string search and replace it with whatever else we need in the future.

So in a nutshell:

document.getElementById()

1) An omni-present function
2) Near-zero informational value
3) Bulky for production; waste of developer time and focus
4) Code-base bloat
5) Bulky code, less readable, less density/compactness
6) Coding style: unnecessary variables, memory footprint, extra lines and complexity in code

$() as a shorthand for document.getElementById()

1) An omni-present function, now with a unique and minimalist face
2) No loss of informational value; it's actually more salient hence easier to spot than before.
3) Simplify production; save developer time and focus
4) Smaller code-base size
5) More readable, dense/compact code
6) Coding style: avoid unnecessary variables, extra lines and code complexity; less memory footprint
7) No maintenance concerns.

I can only say from my own experience when writing up my patches on bug 663695 (e.g. attachment 8919861 [details] [diff] [review]) that using $() is completely fun, a no-brainer time-saver which makes for nicer and simpler code.
Please look at my code there: it's actually a lot easier to spot where elements get initiated because of the unique face of the new $() function.

I hope that explains the "funky thing".
I'm hereby proposing to make this helper function available globally in our codebase (comm-central).

Comment 1

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #0)
> shorthand helper function:
> 
> > var $ = function(aId) {
> >  return document.getElementById(aId);
> > }

This could be rewritten as a normal function; I don't know which is better to share this globally:

function $(aId) {
 return document.getElementById(aId);
}

Comment 2

[Magnus Melin [:mkmelin]]

I'm sorry but I don't think we should do this. 

I don't think there is even necessarily one js file you can define that function in and have it omni present, which will result in bugs when it's suddenly not. NOTE: We don't even globally define Cc/Ci in Thunderbird!!

But importantly also, this is the kind of things that make code less readable to someone not already familiar with the system. document.getElementById is something everybody will know immediately.

So:
> 4) Code-base bloat

Unnecessary helper functions are also bloat.

> 5) Bulky code, less readable, less density/compactness

I think it's more readable, for the reasons stated above. Also, new people would confuse with with jQuery.

> 6) Coding style: unnecessary variables, memory footprint,

I bet you can't introduce the global helper without actually a bunch of footprint increase. Variable definitions are probably optimized away or too small to measure.

Comment 3

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #2)
> I'm sorry but I don't think we should do this. 

It always feels good when new ideas get shot down in the first comment...

> I don't think there is even necessarily one js file you can define that
> function in and have it omni present, which will result in bugs when it's
> suddenly not.

We've got loads of services.jsm etc. etc. are you telling me there isn't one which we could include globally?
That's an implementation detail which isn't a reason against the new implementation itself.

> NOTE: We don't even globally define Cc/Ci in Thunderbird!!

NOTE: We're using loads of helper modules which are sometimes present and sometimes not.
I think everybody must ensure at production time that what he needs is actually present.
And we could just ensure that it's present everywhere.

> But importantly also, this is the kind of things that make code less
> readable to someone not already familiar with the system.
> document.getElementById is something everybody will know immediately.

Oh come on, you are not seriously telling me that it would be hard to figure out the meaning of:
$("attachmentBucket").getItemByIndex[0]

> So:
> > 4) Code-base bloat
> 
> Unnecessary helper functions are also bloat.

You can't be serious, we're talking about a one-line helper function.
We could even hard-code that into every .js file and it would still reduce the size of all .js files if applied more widely.

> > 5) Bulky code, less readable, less density/compactness
> 
> I think it's more readable, for the reasons stated above. Also, new people
> would confuse with with jQuery.

See above, I disagree. That bulky function makes everything around it hard to read, and it uses up a lot of visual estate which distracts from the real meaning of the code. Getting the element is trivial. What's done to the element is what matters. You can't deny that using $() creates more compact code, and smaller file sizes.

> > 6) Coding style: unnecessary variables, memory footprint,
> 
> I bet you can't introduce the global helper without actually a bunch of
> footprint increase. Variable definitions are probably optimized away or too
> small to measure.

We're both guessing. Certainly a 1-line helper function can't cause a "bunch" of footprint increase. A repeatedly used one-line function should probably be optimized away or too small too measure...

Comment 4

[Robert Longson [:longsonr]]

New people coming across this would go, hey I recognise that, that's jQuery. It isn't and would therefore violate the principle of least surprise.

As for your first sentence in comment 3, do read the etiquette guidelines (https://bugzilla.mozilla.org/page.cgi?id=etiquette.html), someone has to make decisions and one day that might be you but that isn't the case today.

Comment 5

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #2)
> > 4) Code-base bloat
> 
> Unnecessary helper functions are also bloat.

Apart from the fact that a one-liner can hardly be bloat:
If we take that argument as an absolute, there's NO helper function which is necessary in a technical sense.
Helper functions are there to help - to make coding easier, and to reduce redundancy and create more compact and more readable code, which is arguably easier to maintain.
There are tons of more or less useful helper functions which according to your argument shouldn't exist then.

Like this one (as an example of a less useful and poorly named helper function)

> function MessageGetNumSelectedAttachments()
> {
>  let bucketList = document.getElementById("attachmentBucket");
>  return (bucketList) ? bucketList.selectedCount : 0;
> }

In many instances, it would actually be clearer AND shorter to just use the original property because you'll probably have bucketList already defined in your context already:

> bucketList.selectedCount

And it's quite unlikey that bucketList doesn't exist (how?) and you've probably checked for that already by the time you want to count selected attachments, or you couldn't even get into that code if there were no attachments.

So this "helper" function isn't very helpful, and yet we're using it all over the place.
By comparison, the helper function which I suggest here is obviously helpful due to its universal code simplifiying effects.

> But importantly also, this is the kind of things that make code less
> readable to someone not already familiar with the system.

After a learning curve of 5 seconds (e.g. for reading a respective comment in our style guide), imo it'll definitely make the entire code more readable as I laid out above with detailed examples.
Plus, if you're saying that helper functions make the code less readable, that applies to any helper function, not just the one which I propose here. Any coder knows that you need to get an element first before you can act on it, so figuring that $(id) gets the element with that ID really can't be hard in any given context, more so if used globally.

Comment 6

[Thomas D. (:thomas8)]

(In reply to Robert Longson [:longsonr] from comment #4)
> New people coming across this would go, hey I recognise that, that's jQuery.
> It isn't and would therefore violate the principle of least surprise.

That's probably the most serious objection, although I'd suspect that it doesn't take more than 5 seconds to realize that it isn't jQuery, more so as there's no trace of jQuery anywhere else.
It's easy to revert globally if we ever want to use jQuery ourselves, but until then, it would just make everyone's life easier and our code more compact. 

We could pick el() or getEl() as a shorthand instead if jQuery is a concern.
I've actually seen getEl() in the Mozilla codebase, for testing.

But anyway, it's not a big thing, just a really existing volunteer contributor (not too many of them out there for TB) trying to get things done faster...

Comment 7

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #2)
> NOTE: We don't even globally define Cc/Ci in Thunderbird!!

Btw, what is Cc/Ci?

Comment 8

[Magnus Melin [:mkmelin]]

(In reply to Thomas D. from comment #3)
> (In reply to Magnus Melin from comment #2)
> > I'm sorry but I don't think we should do this. 
> 
> It always feels good when new ideas get shot down in the first comment...

I don't see how you like it better after 100+ comments ;)
 
> > I don't think there is even necessarily one js file you can define that
> > function in and have it omni present, which will result in bugs when it's
> > suddenly not.

Yes this is how it is. Modules have to be included and even mailServices.js isn't everywhere, but most places. And having it there would not make the expression much/any shorter (you'd still have mailServices.<something>.id() or something of the kind). Making it a globally top function is a no-no.

Comment 9

[Magnus Melin [:mkmelin]]

(In reply to Thomas D. (currently busy elsewhere) from comment #7)
> (In reply to Magnus Melin from comment #2)
> > NOTE: We don't even globally define Cc/Ci in Thunderbird!!
> 
> Btw, what is Cc/Ci?

And the next new contributor would instead ask, what is $.
Components.classes and Components.interfaces

Comment 10

[Thomas D. (:thomas8)]

Apparently I'm neither the first nor the only one with this idea:

https://stackoverflow.com/questions/6398787/javascript-shorthand-for-getelementbyid

(In reply to Magnus Melin from comment #9)
> (In reply to Thomas D. (currently busy elsewhere) from comment #7)
> > (In reply to Magnus Melin from comment #2)
> > > NOTE: We don't even globally define Cc/Ci in Thunderbird!!
> > 
> > Btw, what is Cc/Ci?
> 
> And the next new contributor would instead ask, what is $.

Or maybe they wouldn't ask, because it's very easy to establish from context that $(id) (or $e() / $el() / el() / getEl() ) retrieves the element with the given ID, but you can't guess which special object is hiding behind Cc/Ci.

(In reply to Magnus Melin from comment #8)
> Yes this is how it is. Modules have to be included and even mailServices.js
> isn't everywhere, but most places. And having it there would not make the
> expression much/any shorter (you'd still have mailServices.<something>.id()
> or something of the kind).

Yes, /that/ way wouldn't help at all.

> Making it a globally top function is a no-no.

What makes it so no-no in your opinion?

- We could have $e / $el / el / getEl to eliminate the confusion with jQuery, while still radically simplifying and compacting our code.
- I don't think there's any mentionable performance impact calling a one-liner, and it'll probably be optimized, too.
- If making this globally available from a central place is a problem, we could just include the actual function at the top of every .js file, which isn't much different from including any other module.

I guess that stackoverflow topic exists for a reason: typing document.getElementById, and have that bulky thing all over the place whilst it's not doing anything special is just annoying...

Comment 11

[Philipp Kewisch [:Fallen][📱📆]]

Hi Thomas. Thank you so much for your support in working on patches for Thunderbird. We appreciate your work and the passion you are bringing in. I can understand how the use of $() for getElementById would make things easier, but as you have previously discussed there are also downsides.

Taking all factors into account, Magnus has made the decision that a shorthand such as this one would not be advisable. As he is he module owner for Thunderbird I would appreciate if you could accept this decision. While this may mean a little more typing, I am sure you can configure your editor to autocomplete the function accordingly.

That said, we do appreciate your input and may consider this at a future time if the situation changes. Please keep on asking critical questions in the future, of course keeping the Mozilla CPG https://www.mozilla.org/about/governance/policies/participation/ and bugzilla etiquette in mind.

Comment 12

[:aceman]

(In reply to Thomas D. (currently busy elsewhere) from comment #10)
> Or maybe they wouldn't ask, because it's very easy to establish from context
> that $(id) (or $e() / $el() / el() / getEl() ) retrieves the element with
> the given ID, but you can't guess which special object is hiding behind
> Cc/Ci.

You can, because these are defined at the caller or at the top of each file. They are not hidden in some single file with global definitions. Which is what you propose here.

> (In reply to Magnus Melin from comment #8)
> > Yes this is how it is. Modules have to be included and even mailServices.js
> > isn't everywhere, but most places. And having it there would not make the
> > expression much/any shorter (you'd still have mailServices.<something>.id()
> > or something of the kind).
> 
> Yes, /that/ way wouldn't help at all.

I understand typing document.getElementById is tedious and even error-prone (if you don't have editor with autocompletion). 
I would also love a shorthand. Even mozmill has some controller.e() and .eid() shorthands for this. But there is ugly infrastructure to enable those. We may not have that in main code.
Also document.getElementById() is really fast (may have crazy optimizations in JS/DOM core). Wrapping it inside another function may really add some overhead. Have you tried to benchmark it?

Comment 13

[Thomas D. (:thomas8)]

(In reply to Philipp Kewisch [:Fallen] from comment #11)
> Hi Thomas. Thank you so much for your support in working on patches for
> Thunderbird. We appreciate your work and the passion you are bringing in.

Thanks a lot Philipp for the appreciation and friendly communication style.

> I can understand how the use of $() for getElementById would make things
> easier, but as you have previously discussed there are also downsides.

Not proven, but maybe.

> Taking all factors into account, Magnus has made the decision that a
> shorthand such as this one would not be advisable. As he is he module owner
> for Thunderbird I would appreciate if you could accept this decision.

Acknowledged.

> While this may mean a little more typing,

Apart from typing, it's much more about more compact and readable code.
Typing can be worked around, bulky code with thousands of needless extra lines can't.
We're talking 33,000+ occurences in comm-central.

> I am sure you can configure your editor
> to autocomplete the function accordingly.

Notepad++ is a powerful beast, more so if paired with nppExec plugin.
I succeeded to configure the following ( where | is cursor position)
Ctrl+Shift+4 (Ctrl+$) --> document.getElementById("|")
Alt+Shift+4  (Alt+$)  --> document.getElementById|
:)

> That said, we do appreciate your input and may consider this at a future
> time if the situation changes. Please keep on asking critical questions in
> the future, of course keeping the Mozilla CPG
> https://www.mozilla.org/about/governance/policies/participation/ and
> bugzilla etiquette in mind.

Of course.

Comment 14

[Thomas D. (:thomas8)]

(In reply to :aceman from comment #12)
> (In reply to Thomas D. (currently busy elsewhere) from comment #10)
> > Or maybe they wouldn't ask, because it's very easy to establish from context
> > that $(id) (or $e() / $el() / el() / getEl() ) retrieves the element with
> > the given ID, but you can't guess which special object is hiding behind
> > Cc/Ci.
> 
> You can, because these are defined at the caller or at the top of each file.
> They are not hidden in some single file with global definitions. Which is
> what you propose here.

Not exactly. I also offered another proposal to just add that single-line function at the top of each .js script, if for some reason global inclusion doesn't fly.

> > (In reply to Magnus Melin from comment #8)
> > > Yes this is how it is. Modules have to be included and even mailServices.js
> > > isn't everywhere, but most places. And having it there would not make the
> > > expression much/any shorter (you'd still have mailServices.<something>.id()
> > > or something of the kind).
> > 
> > Yes, /that/ way wouldn't help at all.
> 
> I understand typing document.getElementById is tedious and even error-prone
> (if you don't have editor with autocompletion).

As I said before, autocompletion doesn't help because you still have to type it all the way up to "document.getElementB" until you can just press Enter/Tab to autocomplete. Mouse a bit earlier, but inconvenient.

> I would also love a shorthand.

If you're using Notepad++, I can share my Macros... ;)

> Even mozmill has some controller.e() and
> .eid() shorthands for this. But there is ugly infrastructure to enable
> those. We may not have that in main code.

> Also document.getElementById() is really fast (may have crazy optimizations
> in JS/DOM core). Wrapping it inside another function may really add some
> overhead. Have you tried to benchmark it?

I haven't, and I probably won't, since the decision has already been taken.