3
\$\begingroup\$

I want to see if there's a better way to write this code, solely in Javascript.

There are two kinds of sets, the first set (C7, T7, C11, T11) have four options in the first select, and then up to two options in the second set.

The second set (14X, 17X) have eight options in the first select, and then up to four options in the second set.

Here's the script I've currently got, but it's quite repetitive and long, seems like there could be a better way. Here's the JSFiddle: https://jsfiddle.net/j1fsL9wn/

//Set C7
document.addEventListener("DOMContentLoaded", e => {
  'use strict';

  const combo = document.getElementById("C7-Drawer-Shelf-Combo"),
    bench = document.getElementById("C7-Under-Drawer-Bench");

  combo.addEventListener("input", e => {
    for (let i = 0; i < bench.children.length; i++)
      bench.children[i].hidden = e.target.selectedIndex < 2 && i > e.target.selectedIndex;

    if (e.target.selectedIndex && e.target.selectedIndex < 2 && bench.selectedIndex > e.target.selectedIndex)
      bench.selectedIndex = 0;
  });
});

// Set T7
document.addEventListener("DOMContentLoaded", e => {
  'use strict';

  const combo = document.getElementById("T7-Drawer-Shelf-Combo"),
    bench = document.getElementById("T7-Under-Drawer-Bench");

  combo.addEventListener("input", e => {
    for (let i = 0; i < bench.children.length; i++)
      bench.children[i].hidden = e.target.selectedIndex < 2 && i > e.target.selectedIndex;

    if (e.target.selectedIndex && e.target.selectedIndex < 2 && bench.selectedIndex > e.target.selectedIndex)
      bench.selectedIndex = 0;
  });
});

// Set C11
document.addEventListener("DOMContentLoaded", e => {
  'use strict';

  const combo = document.getElementById("C11-Drawer-Shelf-Combo"),
    bench = document.getElementById("C11-Under-Drawer-Bench");

  combo.addEventListener("input", e => {
    for (let i = 0; i < bench.children.length; i++)
      bench.children[i].hidden = e.target.selectedIndex < 2 && i > e.target.selectedIndex;

    if (e.target.selectedIndex && e.target.selectedIndex < 2 && bench.selectedIndex > e.target.selectedIndex)
      bench.selectedIndex = 0;
  });
});

// Set T11
document.addEventListener("DOMContentLoaded", e => {
  'use strict';

  const combo = document.getElementById("T11-Drawer-Shelf-Combo"),
    bench = document.getElementById("T11-Under-Drawer-Bench");

  combo.addEventListener("input", e => {
    for (let i = 0; i < bench.children.length; i++)
      bench.children[i].hidden = e.target.selectedIndex < 2 && i > e.target.selectedIndex;

    if (e.target.selectedIndex && e.target.selectedIndex < 2 && bench.selectedIndex > e.target.selectedIndex)
      bench.selectedIndex = 0;
  });
});

// Set 14X
document.addEventListener("DOMContentLoaded", e => {
  'use strict';

  const combo = document.getElementById("14X-Drawer-Shelf-Combo"),
    bench = document.getElementById("14X-Under-Drawer-Bench");

  combo.addEventListener("input", e => {
    for (let i = 0; i < bench.children.length; i++)
      bench.children[i].hidden = e.target.selectedIndex < 4 && i > e.target.selectedIndex;

    if (e.target.selectedIndex && e.target.selectedIndex < 4 && bench.selectedIndex > e.target.selectedIndex)
      bench.selectedIndex = 0;
  });
});

// Set 17X
document.addEventListener("DOMContentLoaded", e => {
  'use strict';

  const combo = document.getElementById("17X-Drawer-Shelf-Combo"),
    bench = document.getElementById("17X-Under-Drawer-Bench");

  combo.addEventListener("input", e => {
    for (let i = 0; i < bench.children.length; i++)
      bench.children[i].hidden = e.target.selectedIndex < 4 && i > e.target.selectedIndex;

    if (e.target.selectedIndex && e.target.selectedIndex < 4 && bench.selectedIndex > e.target.selectedIndex)
      bench.selectedIndex = 0;
  });
});

Any advice would be much appreciated!

\$\endgroup\$
1
\$\begingroup\$

As you likely observed in your code, it is very repetitive. Many programmers believe in the Don't Repeat Yourself (i.e. D.R.Y.) principle. One way to do that is to abstract out the common functionality. It seems from your second/related post that there has been a suggestion to abstract the common functionality to a single function that runs for all elements based on a class name.

That abstracted function could be updated to handle the common functionality in the input event callbacks here - namely to:

  • select an associated select list
  • modify the selected value of that select list based on the (index of the) selected value of the list that was changed

In order to determine those items, there are multiple ways of achieving this. One way is to use data attributes. The HTML could be updated so each <select> list has data attributes for the id of the associated select list (i.e. with the under draw bench options) and the threshold.

It seems that jQuery is likely involved now so if that is the case then the .data() method can be used to retrieve data attributes. Otherwise the dataset property can be used to access those attributes.

\$\endgroup\$
1
\$\begingroup\$

Since your code is always repeating the same things with minimum variations, you can put it in function and pass the changing parameters, like so:

function onContentLoaded(code, limit = 2) {
    const combo = document.getElementById(`${code}-Drawer-Shelf-Combo`);
    const bench = document.getElementById(`${code}-Under-Drawer-Bench`);

    combo.addEventListener('input', (e) => {
        for (let i = 0; i < bench.children.length; i++) {
            bench.children[i].hidden = e.target.selectedIndex < limit && i > e.target.selectedIndex;
        }

        if (e.target.selectedIndex && e.target.selectedIndex < limit && bench.selectedIndex > e.target.selectedIndex) {
            bench.selectedIndex = 0;
        }
    });
}
//Set C7
document.addEventListener('DOMContentLoaded', () => onContentLoaded('C7'));

// Set T7
document.addEventListener('DOMContentLoaded', () => onContentLoaded('T7'));

// Set C11
document.addEventListener('DOMContentLoaded', () => onContentLoaded('C11'));

// Set T11
document.addEventListener('DOMContentLoaded', () => onContentLoaded('T11'));

// Set 14X
document.addEventListener('DOMContentLoaded', () => onContentLoaded('14X', 4));

// Set 17X
document.addEventListener('DOMContentLoaded', () => onContentLoaded('17X', 4));
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.