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 Sep 09 '17 at 18:19 — Script47, Sep 09 '17 at 18:19

score 2 · Answer 1 · answered Sep 09 '17 at 18:17

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.

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. — Script47, Sep 09 '17 at 18:22
@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](https://stackoverflow.com/a/46133778/157247)) where it's up-front where it belongs. ([A Macdonald's as well](https://stackoverflow.com/a/46133795/157247), although without the `getDom` function.) — T.J. Crowder, Sep 09 '17 at 18:24

score 1 · Answer 2 · answered Sep 09 '17 at 18:19

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);
}

score 1 · Answer 3 · answered Sep 09 '17 at 18:21

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);
}

DjaouadNM · Answer 4 · 2017-09-09T18:32:41.323

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 09 '17 at 18:32

answered Sep 09 '17 at 18:17

DjaouadNM

22,013
4
33
55

1

Not a good idea at all. This is just ['code smell'](https://stackoverflow.com/questions/5063878/how-to-cleanly-deal-with-global-variables). The better option would be to pass parameters. – Script47 Sep 09 '17 at 18:18
@Script47 It's good for modularity, when you have tens of functions that manipulate the same elements. – DjaouadNM Sep 09 '17 at 18:22
Look at @T.J.Crowder second solution, that would deal with the scenario that you have posed. – Script47 Sep 09 '17 at 18:23
@Script47 It's brilliant, probably my answer could've been better if I moved the variables inside the `.onload`, well, too late. – DjaouadNM Sep 09 '17 at 18:27
1

It's never too late to improve an answer! – Script47 Sep 09 '17 at 18:28
@Script47 Did so. – DjaouadNM Sep 09 '17 at 18:37

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

4 Answers4