JavaScript - function can't acces variables from another function scope

Question

I am trying to create a slider with javscript. I would like to have two functions - first of them, parseDom(), should be responsible for getting elements from DOM; the second one, configureRange(), should be responsible for setting range attributes, like min and max values. Both functions are called inside anonymous function, which is assigned to window.onload variable.

function parseDom() {
  var main = document.getElementById('main');
  main.classList.add('red');
  //   red class added - main selector is ok
  var rangeContainer = main.querySelector('.range-container');
  rangeContainer.classList.add('green');
  //   green class added - rangeContainer selector is ok
  var rangeInput = rangeContainer.querySelector('.range-input');
  rangeInput.classList.add('crosshair');
  //   crosshair class added - rangeInput selector is ok

}

function configureRange(){
  rangeInput.classList.add('pointer');
  rangeInput.setAttribute('min', '0');
}

window.onload = function(){
  parseDom();
  configureRange();
}

However, variables from parseDom() can't be accesed from configureRange(), because variables inside these functions are in different scopes. So my code inside configureRange() does not work. I could do all things in one function instead of two, but this would make code messy. How do I create a good modular solution?

Code is here: https://codepen.io/t411tocreate/pen/oeKwbW?editors=1111

Pass parameters to the functions.

Script47
– Script47

2017-09-09 18:19:24 +00:00
Commented Sep 9, 2017 at 18:19 — Script47
– Script47, Commented Sep 9, 2017 at 18:19

T.J. Crowder · Accepted Answer · 2017-09-09 18:17:35Z

2

The simplest thing is probably to pass configureRange the information it needs, by having parseDom call it:

function parseDom() {
  var main = document.getElementById('main');
  main.classList.add('red');
  //   red class added - main selector is ok
  var rangeContainer = main.querySelector('.range-container');
  rangeContainer.classList.add('green');
  //   green class added - rangeContainer selector is ok
  var rangeInput = rangeContainer.querySelector('.range-input');
  rangeInput.classList.add('crosshair');
  //   crosshair class added - rangeInput selector is ok
  configureRange(rangeInput);             // <==== Added call
}

function configureRange(rangeInput){      // <==== Note new parameter
  rangeInput.classList.add('pointer');
  rangeInput.setAttribute('min', '0');
}

window.onload = function(){
  parseDom();
  //                                         <==== Removed call
}

...or by having a controller function (parseAndConfigure, whatever) that looks up the input and passes it to both functions.

Side note: In terms of keeping functions small and ensuring the name is indicative of what it does (as seems to be your goal), parseDom doesn't parse anything, and it does more than just identify the relevant DOM elements (it also adds classes to them). Perhaps three functions: getDom, addClasses, and configureRange or similar. Then:

window.onload = function() {
    var dom = getDom();
    addClasses(dom);
    configureRange(dom);
}

...or something like that.

answered Sep 9, 2017 at 18:17

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Script47 Over a year ago

The ideal solution would be the second one as it clearly defines what is taking place in the code and removes any chance of clashes with variables.

T.J. Crowder Over a year ago

@Script47: Yeah, my view as well. It really should have been up above that line, not below it, but I'll leave it as-is since I see there are now other answers (like adeneo's) where it's up-front where it belongs. (A Macdonald's as well, although without the getDom function.)

adeneo · Accepted Answer · 2017-09-09 18:19:23Z

You could keep the elements in an object, and then return that object, to be reused anywhere else

function parseDom() {
  var els = (function(d) {
    var main           = d.getElementById('main'),
        rangeContainer = main.querySelector('.range-container'),
        rangeInput     = rangeContainer.querySelector('.range-input');

    return {main, rangeContainer, rangeInput};
  })(document);

  els.main.classList.add('red');
  els.rangeContainer.classList.add('green');
  els.rangeInput.classList.add('crosshair');

  return els;
}

function configureRange(els) {
  els.rangeInput.classList.add('pointer');
  els.rangeInput.setAttribute('min', '0');

  return els;
}

window.onload = function() {
  var elems = parseDom();
  configureRange(elems);
}

A Macdonald · Accepted Answer · 2017-09-09 18:21:44Z

simplest approach would be to abstract the selectors away from the parseDom function, maybe call that updateDom instead and parse the selectors in the top level function e.g.

function updateDom(main, rangeContainer, rangeInput) {
  main.classList.add('red');
  //   red class added - main selector is ok

  rangeContainer.classList.add('green');
  //   green class added - rangeContainer selector is ok

  rangeInput.classList.add('crosshair');
  //   crosshair class added - rangeInput selector is ok

}

function configureRange(rangeInput){
  rangeInput.classList.add('pointer');
  rangeInput.setAttribute('min', '0');
}

window.onload = function(){
  var main = document.getElementById('main'),
    rangeContainer = main.querySelector('.range-container'),
    rangeInput = rangeContainer.querySelector('.range-input');

  updateDom(main, rangeContainer, rangeInput);
  configureRange(rangeInput);
}

Djaouad · Accepted Answer · 2017-09-09 18:32:41Z

0

You could declare your variables inside the .onload, then pass them as arguments to as many functions as you like:

function parseDom(main, rangeContainer, rangeInput) { // <= arguments
  main.classList.add('red');
  rangeContainer.classList.add('green');
  rangeInput.classList.add('crosshair');
}

function configureRange(rangeInput){ // <= argument
  rangeInput.classList.add('pointer');
  rangeInput.setAttribute('min', '0');
}

window.onload = function(){
  var main = document.getElementById('main'),
    rangeContainer = main.querySelector('.range-container'),
    rangeInput = rangeContainer.querySelector('.range-input'),
    // other elements
  parseDom(main, rangeContainer, rangeInput); // <= pass as arguments
  configureRange(rangeInput); // <= pass as argument
}

edited Sep 9, 2017 at 18:32

answered Sep 9, 2017 at 18:17

Djaouad

22.8k4 gold badges37 silver badges57 bronze badges

6 Comments

Script47 Over a year ago

Not a good idea at all. This is just 'code smell'. The better option would be to pass parameters.

Djaouad Over a year ago

@Script47 It's good for modularity, when you have tens of functions that manipulate the same elements.

Script47 Over a year ago

Look at @T.J.Crowder second solution, that would deal with the scenario that you have posed.

Djaouad Over a year ago

@Script47 It's brilliant, probably my answer could've been better if I moved the variables inside the .onload, well, too late.

Script47 Over a year ago

It's never too late to improve an answer!

|

Collectives™ on Stack Overflow

JavaScript - function can't acces variables from another function scope

4 Answers 4

2 Comments

Comments

Comments

6 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related