BJS Bloat and the Multi-Test JS line

[Wingnut]

Hi guys.  A little off topic... probably belongs in the Wingnut Chronicles, but what the heck.

As I tour the Scene Class code, I often see lines like this...

if (rigParent && rigParent.customRenderTargets && rigParent.customRenderTargets.length > 0) {

This entire line... COULD be replaced with...

if(hasSubstance(rigParent.customRenderTargets)) {

A hasSubstance() would do all three tests... 1.  Check if there's a rigParent, 2. Check if rigParent has property .customRenderTargets, 3. Ensure that rigParent.customRenderTargets has at least one member.  Nice, huh?

Let's look at a two-test line...

if (camera.customRenderTargets && camera.customRenderTargets.length > 0) {

A hasSubstance() would work for this, too.

if(hasSubstance(camera.customRenderTargets) {

We saved 43 bytes in the first line, 29 bytes in the second line.

Let's remove a few more bytes, and change hasSubstance() to check().

Check() is obviously a wise idea for bloat reduction, but... what about perf?  Check() is slower, isn't it?

Maybe check() is better named peek()    Add it to JS/ECMA core, eh? 

Hey, all thoughts welcome, and I apologize for being JS-dumb and old-school.  It's just that... these big, fat tri-test code lines seem to happen often in our core code, and if they could be avoided, wouldn't that be sweet?  Everyone with thoughts... please comment... even to call me an idiot.  I am proudly one!  thx.

[Guest]

"if(hasSubstance(rigParent.customRenderTargets)) {" => this cannot test for rigParent and will crash if rigParent is undefined or null

I'm wondering if calling a function will slow things down compare to inline tests. (Well it will slow down but how much?)

[Wingnut]

Yeah, it would need a try/catch... or some other way to return false if ANYTHING went wrong.  hmm.  Thx for thoughts.

I think it needs to be put deep in JS core/internals... but I have no idea what it looks like, there.  Is JS written in machine code?  I'm scared.

[Guest]

JS is the limit we can't go deeper

[Wingnut]

Sure we can.  We can start a giant campaign to rally for a change to ECMAscript, right?  We can gather our troops, and make a stance for truth, justice, and the javascript way.

You got phone numbers and email addresses to Microsoft JS implementers, right DeltaCommando?  Let's make some calls, maybe a movie-trailer-like promo piece... change the JS world forever!

Okay, okay, that's probably too much hoopla.  hmm.  Darn.  SO close to something yummy, yet so far.

[Wingnut]

It would take 10 minutes to add the highly-abusable hasSubstance/peek function to core ECMAscript... which would de-bloat all JS source code worldwide by .001%. 

But, to get that done... would be a miracle.  hehe.  .hasOwnedProperty is close, and so is the has() call in the Underscore.JS add-on lib... but... not quite abusable-enough, yet.

I think the primary issue... is when a property foo.bar == false.  We might try if (hasSubstance(foo.bar)) { ... }  and if hasSubstance() returns false, we would assume that one of the three tests... failed.

When actually, foo.bar DID have substance, and that substance was false.  Perhaps hasSubstance returns undefined if ANYTHING went wrong during the tri-test.  I dunno.  It's all way over my head.... but still...

if (rigParent && rigParent.customRenderTargets && rigParent.customRenderTargets.length > 0)

...is an annoyingly-long line for what it accomplishes. 

Back in the old days... we could do this...

if (var T = hasSubstance(rigParent.customRenderTargets)) { ... }

That is weird, eh?  That's when hasSubstance... becomes peek().

I'm babbling aimlessly... what's new? 

[brianzinn]

You could pass in the property you wanted to avoid dereferencing null/undefined, but it would be quite slow.  Would be cool if the typescript compiler had an inline annotation like in C.  This is not tested at all.
called like: hasSubstance(rigParent, 'customRenderTargets')

@inline('no such annotation')
hasSubstance(obj: any, prop: String) {
  if (!obj) {
    return false;
  }

  if (obj[string] === undefined || !Array.isArray(obj[string])) { // otherwise do a falsey test
    return false;
  }

  return obj[string].length > 0;
}

Fun thought experiment.  Sounds like speed wins over readability/code reuse!  I had some similar thoughts when looking at some duplicate code in VR rigs as the inline code seems harder to maintain.
edit: the above code is maybe even harder to maintain, as it won't refactor or give compile errors when ie: property is renamed.

edit2: wonder about using generics and lambda as function need only be called if obj is non-null and defined -- hasSubstance(obj: T, (a: T) => a.customRenderTargets... :)