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

Unclear code example in variables section #136

Closed
Daniel-Hug opened this issue Jan 6, 2014 · 9 comments · May be fixed by iamstoick/javascript#3
Closed

Unclear code example in variables section #136

Daniel-Hug opened this issue Jan 6, 2014 · 9 comments · May be fixed by iamstoick/javascript#3

Comments

@Daniel-Hug
Copy link

At the bottom of the variables section, this code example is given:

// bad
function() {
  var name = getName();

  if (!arguments.length) {
    return false;
  }

  return true;
}

// good
function() {
  if (!arguments.length) {
    return false;
  }

  var name = getName();

  return true;
}

At the top of that code block it says:

Assign variables at the top of their scope. This helps avoid issues with variable declaration and assignment hoisting related issues.

It's not clear what point the code example is trying to make, as it doesn't reflect the statement made in that comment. Plus, the name variable is defined but never used.

@nmussy
Copy link
Contributor

nmussy commented Jan 6, 2014

I'm not sure about unclear, but what I saw hear was a counterexample of "Assign variables at the top of their scope."

Here, if the test is false, we'd have at least one useless declaration, function call and assignment.
Maybe sectioning this chunk of code and explaining it beforehand would be best?

@yaroslavya
Copy link

Variable will be hoisted anyway. But getName will not be executed.

I would go with an alternative example: https://gist.github.com/yaroslavya/8957634
its starwars oriented, so not sure if its better to use simple foo, bar.

@ghost
Copy link

ghost commented Sep 9, 2014

+1

@ghost
Copy link

ghost commented Sep 9, 2014

This does seem like a counterexample. E.g.,

whereas in the former example,

// bad
function() {
  test();
  console.log('doing stuff..');

  //..other stuff..

  var name = getName();

  if (name === 'test') {
    return false;
  }

  return name;
}

// good
function() {
  var name = getName();

  test();
  console.log('doing stuff..');

  //..other stuff..

  if (name === 'test') {
    return false;
  }

  return name;
}

The variable name is declared before an if block using name is declared.

In the "counterexample", whereas here the if block uses arguments inherent to the function, then var name = ... is not declared until afterwards. I guess it also makes sense since if the if block passes, it will return false and will never have to declare name in the first place -- saving a few steps.

@agustin107
Copy link

All variables was declared at the start function, because for this way, we avoid the hoisting problem :).
Sorry for my English, bye 👍

@rvarbanov-old
Copy link

The reason why you place

  if (!arguments.length) {
    return false;
  }

before the veritable declaration

var name = getName();

is that the function is expecting arguments that are probably required by the getName(). If not arguments are supplied the function will exit and the var name = getName(); will not be executed.

I think the confusion here comes from the over simplified example.

You always want to declare your variables on top of the function block, but if your function depends on an argument you want to skip the execution of any code if that argument requirement is not met.

@nkbt
Copy link

nkbt commented Sep 11, 2014

You can always do

var name;
if (!arguments.length) {
    return false;
} 
name = getName();

@ghost
Copy link

ghost commented Sep 12, 2014

... I believe we are all thinking and saying the same things now in different words. Can someone submit a pull request? (too lazy/busy here to be the one :) ).

jakeoverall added a commit to jakeoverall/javascript that referenced this issue Mar 30, 2015
Unclear code example in variables section. Variables should be declared at the top of their scope to avoid declaration hoisting.
@goatslacker
Copy link
Collaborator

Closing in favor of #265

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 a pull request may close this issue.

7 participants