11
\$\begingroup\$

I will ask my question using the following example:

HTML

<div>
    <input id="ABC" placeholder="abc"></input>
    <textarea id="XYZ" placeholder="xyz"></textarea>
</div>  

JS Code

var valArray = [{id: "ABC", value: "INPUT"}, {id: "UNKNOWN", value: ""}, {id: "XYZ", value: "TEXTAREA"}]

function process(arr) {
    for(var i=0; i<arr.length; i++) {
        var elem = document.getElementById(arr[i].id);
        if (elem != null) {
            elem.value = arr[i].value;
        }
    }   
}

process(valArray);  

It works fine, but now I want to do it without an explicit if statement.

How about the following code? Is it a proper JavaScript way to handle such situations?

function process(arr) {
    for(var i=0; i<arr.length; i++) {
        (document.getElementById(arr[i].id) || 0).value = arr[i].value;
    }   
}

From Programming JavaScript Applications by Eric Elliott- Chapter 3. Objects:

Even primitive types get the object treatment when you refer to them with the property access notations. They get automatically wrapped with an object so that you can call their prototype methods.

Primitive types behave like objects when you use the property access notations, but you can't assign new properties to them. Primitives get wrapped with an object temporarily, and then that object is immediately thrown away. Any attempt to assign values to properties will seem to succeed, but subsequent attempts to access that new property will fail.

In general I saw a lot of JavaScript code where || <default or alternative value> construct was used to protect against null.

So I thought, that the above described behavior would make primitives ideal candidates for "throw-away" objects.

If this is so convoluted, what about using Object:

        (document.getElementById(arr[i].id) || Object).value = arr[i].value;

That was my original idea but it seems to pollute the environment.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ How could you wonder if a cyphered code can be preferred to the most simple ’if (x != null )’ ??? \$\endgroup\$ Sep 28 '14 at 15:56
  • \$\begingroup\$ @GameAlchemist - Please see the additional information I posted. \$\endgroup\$
    – PM 77-1
    Sep 28 '14 at 17:02
5
\$\begingroup\$

I wondered why one could even consider testing against null like this, then i found the 'trick' :

You are using this pattern only once.

Now in a real-world example where you have to test several objects, the mess effect will raise at an incredible rate.

Imagine the not so complex app init : initialize all controls -if found- with the value of another control -if found- otherwise with a default value.

var initMap = [ { tgt : 'display1', src : 'input1', dft : '0'     }, 
                { tgt : 'display2', src : 'inputB', dft : 'city'  }
                /* ... */
              ];

var $ = document.getElementById.bind(document);

function initControls(initMap) {
    for (var i=0; i<initMap.length; i++) {
        var thisInitData = initMap[i];
        var tgtElement   = $(thisInitData.tgt);
        var srcElement   = $(thisInitData.src);
        var dft          = thisInitData.dft;
        (tgtElement || 0).value = ( ( srcElement || 0 ).value || default );  // ?? wth ?
    }        
}

Now i call the last line cryptic programming :)

I consider the following code much clearer about the intent : if there's a target, then either assign this or that value to the element depending on source being null.

function initControls(initMap) {
    for (var i=0; i<initMap.length; i++) {
        var thisInitData   = initMap[i];
        var tgtElement     = $(thisInitData.tgt);
        if (tgtElement !== null) {
            var srcElement = $(thisInitData.src);
            tgtElement.value = (srcElement !== null) ? srcElement.value 
                                                     : thisInitData.dft ;
        }
    }        
}

And if you are still not convinced, build an example with 3 or more tests.

\$\endgroup\$
12
\$\begingroup\$
(document.getElementById(arr[i].id) || 0).value = arr[i].value;

In this code there is a pointless assignment when the element is not found. In this case, the 0 gets wrapped into a temporary object, and the assignment will work, but it will be pointless, it's result will be thrown away. This assignment is merely a side effect of the shortcut, it has no purpose of its own.

Every operation in a program should be intentional, purposeful, and this shortcut violates that. As such, I consider this a dirty hack. Don't use it.


When in doubt, go for the more intuitive and readable solution. The first example in your post is slightly longer, but it's simple and clear: there's no question about what will happen.

The second way, although shorter, may not be obvious for everyone.

Keep in mind that code is read far more times than it’s written. It's better to favor a technique that's clear and easy to understand to something "clever" but potentially confusing / obscure.


Additional excellent recommendation by @GameAlchemist, verbatim:

1) for optimisation purposes, the if is faster. and more importantly 2) a piece of code should be as much self-documenting as possible. Even if (element) is better written if (element != null) (which makes obvious the defensive check against getElementById returning null)

\$\endgroup\$
8
  • \$\begingroup\$ Why do you consider this hack dirty? Do you believe that future ECMA standards or implementations will make it invalid? \$\endgroup\$
    – PM 77-1
    Sep 28 '14 at 18:07
  • 3
    \$\begingroup\$ I consider it dirty because it uses a pointless assignment. The pointless assignment, although harmless, it's not intended, it's just a side-effect of the shortcut. Every operation in a program should be intentional, purposeful, and this shortcut violates that. \$\endgroup\$
    – janos
    Sep 28 '14 at 18:11
  • 2
    \$\begingroup\$ I would add that 1) for optimisation purposes, the ‘if‘ is faster. and more importantly 2) a piece of code should be as much self-documenting as possible. Even ‘if (element)‘ is better written ‘if (element != null)‘ (which makes obvious the defensive check against getElementById returning null) . \$\endgroup\$ Sep 28 '14 at 21:16
  • \$\begingroup\$ Thanks @GameAlchemist, excellent points! (added to my post) \$\endgroup\$
    – janos
    Sep 28 '14 at 21:29
  • 2
    \$\begingroup\$ Good answer, but I generally prefer if (element) as a compact & popular Javascript idiom. Not only is it shorter, it guards against undefined as well. \$\endgroup\$
    – Thomas W
    Sep 29 '14 at 1:44
8
\$\begingroup\$

Can you do this?

Yes, you can.

When to do this?

Everywhere, except collaboration purposes.

Why?

(document.getElementById(arr[i].id) || 0).value = arr[i].value;

and

(document.getElementById(arr[i].id) || Object).value = arr[i].value;

tells nothings about what it does when you read this first time, because there are two logical parts:

  • retriving element and replacing null with default value
  • setting attribute value

You can write long one line code only if it is separate logic part.

BTW: I'd wrote something like this

function process(arr) {
    arr.forEach(function(item){
        var elem = document.getElementById(item.id) || 0;
        elem.value = item.value;
    });
}

Because programming language is tools to express your thoughts.

\$\endgroup\$
5
\$\begingroup\$

Just don't. The sole benefit of your compact version is that it is more compact. It is neither faster nor more readable. As you are aware, it has a weird side-effect of altering 0. Even if you eliminated the side-effect (by setting ({}).value, for example), I would still strongly prefer the original code, which is much more straightforward.

In addition, process(arr) is an extremely generic, uninformative name for a function. setFormValues(idValueMaps) would be a much better name.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.