CODEWITHSUNDEEP
javascriptjquery
killerprince182
killerprince182

Reputation: 465

How can I refactor the code to avoid global scope?

I have created a sliding menu with js and css. "menufull" checks for the current status. If it's in full width then it is true, otherwise false.

However, I want to check for the status in togglemenu function itself. if I write menuFull = false in a function, it will be reset every time I enter this function. What's the better way to write this? I want to avoid global variables as much as possible.

//checks for the status of menu
let menuFull = false;

$(document).ready(function () {
    //Handle navigation button
    let navButton = document.getElementById("nav_button");
    navButton.addEventListener('click', function (event) {
        toggleMenu();
    })
});

let toggleMenu = function () {
    //Check for the status here
    let menuArea = document.getElementById("nav_container");
    let containerArea = document.getElementsByClassName("body_container")[0];

    if (menuFull) {
        menuArea.style.left = "-11.5%";
        menuFull = false;
        console.log("I am in true area!");
    } else {
        menuArea.style.left = "0%";
        containerArea.style.left = "18.5%";
        menuFull = true;
        console.log("I am in false area!")
    }

    console.log(menuFull);
}

Upvotes: 0

Views: 54

Answers (1)

CertainPerformance
CertainPerformance

Reputation: 371233

Easiest tweak would be to just put the whole thing inside an IIFE:

(() => {
    //checks for the status of menu
    let menuFull = false;

    $(document).ready(function () {
        //Handle navigation button
        let navButton = document.getElementById("nav_button");
        navButton.addEventListener('click', function (event) {
            toggleMenu();
        })
    });

    let toggleMenu = function () {
        //Check for the status here
        let menuArea = document.getElementById("nav_container");
        let containerArea = document.getElementsByClassName("body_container")[0];

        if (menuFull) {
            menuArea.style.left = "-11.5%";
            menuFull = false;
            console.log("I am in true area!");
        } else {
            menuArea.style.left = "0%";
            containerArea.style.left = "18.5%";
            menuFull = true;
            console.log("I am in false area!")
        }

        console.log(menuFull);
    }
})();

Because it's only used inside of toggleMenu, you could also call an IIFE to create the toggleMenu function, with menuFull scoped only inside it:

$(document).ready(function () {
    //Handle navigation button
    let navButton = document.getElementById("nav_button");
    navButton.addEventListener('click', function (event) {
        toggleMenu();
    })
});

let toggleMenu = (() => {
    //checks for the status of menu
    let menuFull = false;
    return () => {
      //Check for the status here
      let menuArea = document.getElementById("nav_container");
      let containerArea = document.getElementsByClassName("body_container")[0];

      if (menuFull) {
          menuArea.style.left = "-11.5%";
          menuFull = false;
          console.log("I am in true area!");
      } else {
          menuArea.style.left = "0%";
          containerArea.style.left = "18.5%";
          menuFull = true;
          console.log("I am in false area!")
      }

      console.log(menuFull);
  };
})();

toggleMenu is still on the top level here, though. If you don't want that, you can define it all inside of the $(document).ready:

$(document).ready(function () {
    let toggleMenu = (() => {
        // ...
    })();
    $('#nav_button').on('click', toggleMenu);
});

Since you're already using jQuery, you should probably use jQuery's (concise) method of selecting elements and adding listeners rather than using the native DOM methods.

Upvotes: 1

Related Questions