Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typos and closure that wasn't a closure #1097

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rodrigograca31
Copy link

@rodrigograca31 rodrigograca31 commented Oct 18, 2019

I'd like to have this PR accepted, it fixes a few typos and wrong code.

Should close #1009

}
myFunction();
const closureFunction = myFunction();
closureFunction();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both versions of the code seem to work. This exercise deals in lexical scoping. Do you think the changes proposed make the rules of visibility clearer? Is this what you refer to as the "closure that wasn't"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So........ The problem is a bit hard to explain. Yes the function works and yes it can access the lexical scope but it's not a closure like it states above.
It can access it because was declared first. not because its a closure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All functions in JS are closures! They all "remember" the bindings in the environment they were defined in, even when that environment has gone away (returned). Your version makes the "even" part super explicit. Let's come up with another name for "closureFunction"! This way we don't give ideas that closures have to be built "just so". Something that makes the point that "closureFunction" points exactly to "nestedFunction". Can you think of anything? Something like "nestedFunctionAlso"? Mh. I don't like that...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is that the current function works because internal is declared before nestedFunction so its a normal variable being declared before a normal function. If we had a counter inside it would not increment because it would not keep a reference to it in memory.

This is what I mean: https://stackoverflow.com/a/35130876/1368742
The current function is just a "function with free variables".

@@ -8,14 +8,14 @@


/* == Step 2: Volume Method ==
Create a method using CuboidMaker's prototype that returns the volume of a given cuboid's length, width, and height
Create a method named volume using CuboidMaker's prototype that returns the volume of a given cuboid's length, width, and height
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Closure example not a closure
2 participants