Script becomes too big-CodePudding

I'm trying to optimize the code I have.

Here is an example of how I use it. If I need to increase the number of buttons on the main page from 3 to 4 I follow these steps:

In HTML I will add another button with onclick="one(4)".
In JS I will create another array called test4 and include some data there.
In function one I will add another if with number === 4.
In function demo1 I will add 3 more lines with if size === 10, size === 11 and size === 12.

Eventually, the file will become huge and the numbers will be very big too because I want to add a lot of buttons on the main page.

Can you please advise if there is a way to simplify the code?

Here is the snippet:

let test1 = ["11", "12", "13"];
let test2 = ["21", "22", "23"];
let test3 = ["31", "32", "33"];


function one (number) {
if (number === 1) {
    document.getElementById("btn").classList.remove("hidden")
    document.getElementById("spis").classList.add("hidden")
    document.getElementById("demo").classList.remove("hidden")
    document.getElementById("btn1").onclick = function() {demo1(1)}; 
    document.getElementById("btn2").onclick = function() {demo1(2)}; 
    document.getElementById("btn3").onclick = function() {demo1(3)}; 
    }
if (number === 2) {
    document.getElementById("btn").classList.remove("hidden")
    document.getElementById("spis").classList.add("hidden")
    document.getElementById("demo").classList.remove("hidden")
    document.getElementById("btn1").onclick = function() {demo1(4)}; 
    document.getElementById("btn2").onclick = function() {demo1(5)}; 
    document.getElementById("btn3").onclick = function() {demo1(6)}; 
    }
if (number === 3) {
    document.getElementById("btn").classList.remove("hidden")
    document.getElementById("spis").classList.add("hidden")
    document.getElementById("demo").classList.remove("hidden")
    document.getElementById("btn1").onclick = function() {demo1(7)}; 
    document.getElementById("btn2").onclick = function() {demo1(8)}; 
    document.getElementById("btn3").onclick = function() {demo1(9)}; 
    }    
}


    
function demo1 (size) {
document.getElementById("demo").classList.remove("hidden")
    if (size === 1) {
        document.getElementById("demo").innerText = test1[0];
    }
    if (size === 2) {
        document.getElementById("demo").innerText = test1[1];
    }
    if (size === 3) {
        document.getElementById("demo").innerText = test1[2];
    }
    if (size === 4) {
        document.getElementById("demo").innerText = test2[0];
    }
    if (size === 5) {
        document.getElementById("demo").innerText = test2[1];
    }
    if (size === 6) {
        document.getElementById("demo").innerText = test2[2];
    }
    if (size === 7) {
        document.getElementById("demo").innerText = test3[0];
    }
    if (size === 8) {
        document.getElementById("demo").innerText = test3[1];
    }
    if (size === 9) {
        document.getElementById("demo").innerText = test3[2];
    }    

}

function three () {
    document.getElementById("btn").classList.add("hidden")
    document.getElementById("spis").classList.remove("hidden")
    document.getElementById("demo").classList.add("hidden")
    document.getElementById("demo").innerText = "";
}

.active {
    display: block;
}

.hidden {
    display: none;
}

.column {
  display: flex;
  flex-direction: column;
}

        <div  id="btn">
        <button id="btn1">1</button>
        <button id="btn2">2</button>
        <button id="btn3">3</button>
        <button onclick="three()">Back</button>
        </div>
        <div id="spis"> 
        <div >
        <button onclick="one(1)">1</button>
        <button onclick="one(2)">2</button>
        <button onclick="one(3)">3</button> 
        </div>
        </div>
        <p id="demo"></p>

CodePudding user response：

i would remove the if conditions first and move to switch cases because im my opinion its just not very good to maintain a code like that. also all those dom manipulations. i would move them to different functions and pass the changes as arguments this will make the code more clean and easy to maintain

because in a case if you change one class name you have to change all those dom manipulations i dont think that will be easy to maintain. and in my opinion that's not very good. practice.

here's the code i did some modifications for you.

let test1 = ["11", "12", "13"];
let test2 = ["21", "22", "23"];
let test3 = ["31", "32", "33"];

function one(number) {
  switch (number) {
    case 1:
      hideThings();
      buttonFunctions(1, 2, 3);
      break;
    case 2:
      hideThings();
      buttonFunctions(4, 5, 6);
      break;
    case 3:
      hideThings();
      buttonFunctions(7, 8, 9);
      break;
    default:
      console.log("error");
      break;
  }
}

function demo1(size) {
  document.getElementById("demo").classList.remove("hidden");

  switch (size) {
    case 1:
      ChangeDemoState(0);

      break;
    case 2:
      ChangeDemoState(1);

      break;
    case 3:
      ChangeDemoState(2);

      break;
    case 4:
      ChangeDemoState();

      break;
    // do the rest here
    default:
      console.log("error");
  }
}

const hideThings = () => {
  document.getElementById("btn").classList.remove("hidden");
  document.getElementById("spis").classList.add("hidden");
  document.getElementById("demo").classList.remove("hidden");
};

const ChangeDemoState = (value) => {
  document.getElementById("demo").innerText = test3[value];
};

const buttonFunctions = (v1, v2, v3) => {
  document.getElementById("btn1").onclick = function () {
    demo1(v1);
  };
  document.getElementById("btn2").onclick = function () {
    demo1(v2);
  };
  document.getElementById("btn3").onclick = function () {
    demo1(v3);
  };
};

function three() {
  document.getElementById("btn").classList.add("hidden");
  document.getElementById("spis").classList.remove("hidden");
  document.getElementById("demo").classList.add("hidden");
  document.getElementById("demo").innerText = "";
}

CodePudding user response：

Kind of non-deep refactoring:

const tests = {
  1: ["11", "12", "13"],
  2: ["21", "22", "23"],
  3: ["31", "32", "33"],
  4: ["41", "42", "43"],
};

const elements = {
   btn: document.getElementById("btn"),
   spis: document.getElementById("spis"),
   demo: document.getElementById("demo"),
   btnCol1: document.getElementById("btn1"),
   btnCol2: document.getElementById("btn2"),
   btnCol3: document.getElementById("btn3"),
}

const demo1 = (e, row) => {
  const col = e.dataset.col - 1;
  const result = tests[row][col];
  elements.demo.innerText = result;
}

function one(e) {
    elements.btn.classList.remove("hidden");
    elements.spis.classList.add("hidden");
    elements.demo.classList.remove("hidden");
    
    const row = e.dataset.row;
    elements.btnCol1.onclick = function() {demo1(this, row)}; 
    elements.btnCol2.onclick = function() {demo1(this, row)}; 
    elements.btnCol3.onclick = function() {demo1(this, row)};  
}

function three() {
    elements.btn.classList.add("hidden")
    elements.spis.classList.remove("hidden")
    elements.demo.classList.add("hidden")
    elements.demo.innerText = "";
}

.active {
    display: block;
}

.hidden {
    display: none;
}

.column {
  display: flex;
  flex-direction: column;
}

<div  id="btn">
  <button id="btn1" data-col="1">1</button>
  <button id="btn2" data-col="2">2</button>
  <button id="btn3" data-col="3">3</button>
  <button onclick="three()">Back</button>
</div>
<div id="spis">
  <div >
    <button onclick="one(this)" data-row="1">1</button>
    <button onclick="one(this)" data-row="2">2</button>
    <button onclick="one(this)" data-row="3">3</button>
    <button onclick="one(this)" data-row="4">4</button>
  </div>
</div>
<p id="demo"></p>