Skip to content

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

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.

Sign up for GitHub

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

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18510ae
Fix typos and closure that wasn't a closure
rodrigograca31 Oct 18, 2019
f0d88af
Back to function name
rodrigograca31 Oct 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions challenges/functions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

// ==== Closures ====

// Explain in your own words why nestedfunction can access the variable internal.
// Explain in your own words why "nestedFunction" can access the variable internal.

// Explanation:

Expand All @@ -37,6 +37,7 @@ function myFunction() {
function nestedFunction() {
console.log(internal);
};
nestedFunction();
return nestedFunction;
}
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".

2 changes: 1 addition & 1 deletion challenges/objects-arrays.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Log the result of your new array. */
const contactInfo = [];
console.log(contactInfo);

/* Request 3: Find out how many universities have the string "Uni" included in their name. Create a new array called unisWithUni that contains them all. This will be an array of objects. Log the result. */
/* Request 3: Find out all universities that have the string "Uni" included in their name. Create a new array called unisWithUni that contains them all. This will be an array of objects. Log the result. */
const unisWithUni = [];
console.log(unisWithUni);

Expand Down
4 changes: 2 additions & 2 deletions challenges/prototypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.


Formula for cuboid volume: length * width * height
*/


/* == Step 3: Surface Area Method ==
Create another method using CuboidMaker's prototype that returns the surface area of a given cuboid's length, width, and height.
Create a method named surfaceArea using CuboidMaker's prototype that returns the surface area of a given cuboid's length, width, and height.

Formula for cuboid surface area of a cube: 2 * (length * width + length * height + width * height)
*/
Expand Down