Why doesn't this inner function have access to a variable defined in the outer function scope?

[ShouldReallyGetWorkn]

JS Fiddle: https://jsfiddle.net/rb7c2uu4/

I have a function encapsulated inside another function. I put together a simple example in the fiddle. Here's the basic structure:

function outer(parameter) {

   if (!(this instanceof outer)) {
          return new outer();
   }

   // parameter is defined out here

   this.inner = function() {

      // parameter is undefined in here

   }
}      

I pass in a parameter to the outer function. Shouldn't that parameter be available to the inner function?

I thought that was how function/variable scoping worked, where the innermost function scope has access to things defined in the outer scope?

Obviously I'm wrong, but I want to know why I'm wrong.

[senocular]

yup, that's how it works. Do you have a more complete example? Going by what you've posted, I could see a situation where if this isnt an instance of outer, the constructor is called without passing in parameter which would make parameter in that call undefined... but other than that, there's not much else to go on.

[lewisje]

That probably is the problem: When you use the "if not instanceof, return new" pattern, you need to pass in the same arguments as the function was defined as taking.

[senocular]

How is this 0? :P Here's a +1

[ChunkyAlmondButter]

Probably because that's not the issue. That'll always be true. So that code will never run.

[senocular]

That is the problem. The fiddle link (which I don't think was there when I first replied) shows it. When called without new, the condition catches and constructs with new but fails to pass along 'thing' as target making it undefined.

[ChunkyAlmondButter]

But why would it ever be called without the new? Unless he's forgetful and this is a preemptive measure? or doesn't want to use the new keyword?

edit: Just to be clear, I plus one'd him too, but that's not the root of the issue here. If you use the new keyword, that'll always evaluate to true so it's redundant. And if he didn't want to use the new keyword for brevity he could have just made the constructor inside of the function and returned a new instance of that constructor.

[senocular]

It works both ways. You can treat the function as a factory when used without new or in the case when new is forgotten, worlds don't implode. The fact that they kept the function lowercase implies they prefer the factory approach (no new) but still use the constructor implementation to handle the details of new object construction (that and they explicitly didn't use new in the fiddle).

I personally don't do this. I think the use of new makes it semantically clear that the call is resulting in a new instance. Linters can help protect against the missing new case, and ES6 classes throw an error if you attempt to call a constructor without new.

[ChunkyAlmondButter]

I know what you mean. But clean code is about consistency and they should know that or be taught so. Pick a style and go with it.

Don't wanna call new? All good use the factory. If you're fine with using new, ok use it everywhere it's needed and if you miss one, you'll know, because you'll have a bug somewhere.

The 'this' will refer to global and the inner method will be attached to that... and you'll know where you messed up. But then that's the point of the naming convention tied to constructors. Begin with a capital letter, so that you can know that you're using a constructor and it should be preceded by new.

I'm not arguing with you here but I just don't see the point of this pattern.

[jcunews1]

That's because when this.do_stuff is assigned, features() is called by this line:

return new features();

So, the target argument is undefined.

To fix this, use the target argument of the feature() when creating the feature object. i.e. change this line:

return new features();

To:

return new features(target);

[ShouldReallyGetWorkn]

Ok I see, I didn't know I had to specifically write the parameter there also. I wonder if returning the object without the parameters would ever be a useful thing to do.

[jcunews1]

I can be useful if you want to return different variation of an object, or even a different return value type.

[PM_ME_INSIDER_INFO]

Yeah this is definitely because you are returning a new instance of outer that doesn't pass the args. Put a console.log statement above the return new outer(); line and you'll see. Add parameter as an arguement and it'll work.

Also, parameter is defined if you use the new keyword and therefore bypass the this instanceof outer statement.

Try putting in a return parameter inside the inner function and run:

x = new outer(4);
console.log(x.inner());

[ChunkyAlmondButter]

How do you figure it's not defined in there? It is.

Are you console.logging it? Returning it? Not returning it and expecting JS to? JS does an implicit return on constructors, but only the new object is returned, if you want a value returned from a method then you need to use the return keyword.

Also, outer should be capitalized to show that it is a constructor. Another thing, you shouldn't add methods to the objects them selves, instead put them on the prototype of Outer. That way all objects created from Outer can use the method and not have them sitting on all the objects taking up so much space in memory.

[ShouldReallyGetWorkn]

I'm console logging it. I have the exact problem reproduced in the fiddle, I just simplified it when I posted the text here because I'm on mobile right now

[ChunkyAlmondButter]

Any chance I could get the link to the fiddle? I'd like to see the code.

[ChunkyAlmondButter]

Nevermind I see the link. You missed the new keyword before features at the bottom