Use JS function in place of repeating code

Question

I know this should be easy.. but im still learning JS :)

this code works fine, but currently I will have to repeat this about 20 times, just changing the L50C object, and the xp_50...

let L50C = document.getElementById('lvl50');
chrome.storage.sync.get('L50color', function(data) {L50C.setAttribute('value', data.L50color);
});
let L60C = document.getElementById('xp_60');
chrome.storage.sync.get('L60color', function(data) {L60C.setAttribute('value', data.L60color);
});

L40C.onchange = function(element) {
    chrome.tabs.query({active: true, currentWindow: true}, function(tabs) {
      chrome.tabs.insertCSS(
          tabs[0].id,
          {code: ".xp_40 {border: 1px solid " + element.target.value + " !important;} .xp_40 .icon {color: " + element.target.value + " !important;}"});
    });
  };
L50C.onchange = function(element) {
    chrome.tabs.query({active: true, currentWindow: true}, function(tabs) {
      chrome.tabs.insertCSS(
          tabs[0].id,
          {code: ".xp_50 {border: 1px solid " + element.target.value + " !important;} .xp_50 .icon {color: " + element.target.value + " !important;}"});
    });
  };

(im using #60 to test) so.. if I change the id="xp_60" like this...

<div class="lvl_inp_cont">Level 60: <input id="xp_60" type="text" class="lvl_input" maxlength="7"></div>

can i call a function to repeat... like this:

L60C.onchange = swap(element);

function swap(element) {
    chrome.tabs.query({active: true, currentWindow: true}, function(tabs) {
      chrome.tabs.insertCSS(
          tabs[0].id,
          {code: "." + element.id + " {border: 1px solid " + element.target.value + " !important;} ." + element.id + " .icon {color: " + element.target.value + " !important;}"});
    });
  };

see so instead of all that code, i can just set it to one re-usable code? I feel like the swap function looks right (if not i can fix it) - its just how do I "call" or set the onchange to that function?

Thanks!!

EDIT: am i close?

const obj = {L40C, L50C, L60C, L70C};

for(let num = 40; num < 150; num+=10) {
    let obj['L' + num + 'C'] = document.getElementById('xp_' + num);
}

added another (L40C), and showed where I am setting the L40C.. - yes the xp_ will be the naming element because it would match what I am altering in the CSS so I could, as show, grab the ID of xp_XX and use it along with the value (in the textbox) - or is there also a more efficient way to fo the let L40C = doc... — StaticFX
– StaticFX, Commented Oct 26, 2018 at 17:10
L60C.onchange = swap(element); is calling the method right away as you may have seen. When you set an event handler the first argument for the handler (which you have as element in both examples is actually the event. event.target has the reference to the element that triggered the event. So setting L60C.onchange = swap; is closer to what you want, but you'll still have to make some changes. — lemieuxster
– lemieuxster, Commented Oct 26, 2018 at 17:13
So L50C refers to #lvl50 and L60C refers to #xp_60, and not, for example, to #lvl60? That'll make DRY code harder — CertainPerformance
– CertainPerformance, Commented Oct 26, 2018 at 17:18
Also, .xp_40 refers to a class name of xp_40 (in the upper code), but in the bottom you're concatenating with "." + element.id, which is an id, not a className? (also element in your lower code actually refers to the event, not the element, see answer) — CertainPerformance
– CertainPerformance, Commented Oct 26, 2018 at 17:21
yep the xp_60 was just a test (I am going to change all if it works), and.. yeah I caught that element.id should be element.target.id thanks! — StaticFX
– StaticFX, Commented Oct 26, 2018 at 17:34

CertainPerformance · Accepted Answer · 2018-10-26 17:16:58Z

1

It looks like the numbers, like the 40 in L40C.onchange are what change each time, so I'd create an array of all such numbers, then iterate over them and assign the appropriate chrome.tabs.query. But you'll need an object or array of the LC40, LC50s, rather than tens of separate variable names - for example:

const obj = { L40C, L50C, ... };

Then, you can use variables for property access, and instead of your L40C.onchange and L50C.onchange, you could do:

['40', '50'].forEach((num) => {
  const className = 'xp_' + num;
  obj['L' + num + 'C'].onchange = ({ target }) => {
    chrome.tabs.query({active: true, currentWindow: true}, (tabs) => {
      chrome.tabs.insertCSS(
        tabs[0].id,
        {code: "." + className + ' {border: 1px solid " + target.value + " !important;} .' + className + ' .icon {color: ' + target.value + ' !important;}'}
       );
    });
  };
});

It's important to note that the parameter to the onchange handler is an event, not an element. event.target refers to the element that triggered the event.

answered Oct 26, 2018 at 17:16

CertainPerformance

373k55 gold badges354 silver badges359 bronze badges

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

1 Comment

StaticFX Over a year ago

I like you 2nd example (for ease of use..) i will give it a shot!

Stramski · Accepted Answer · 2018-10-27 08:37:32Z

0

So you should just need to set:

function swap(element) {
    chrome.tabs.query({active: true, currentWindow: true}, function(tabs) {
      chrome.tabs.insertCSS(
          tabs[0].id,
          {code: "." + element.id + " {border: 1px solid " + element.target.value + " !important;} ." + element.id + " .icon {color: " + element.target.value + " !important;}"});
    });
  };

L40C.onchange = swap;
L50C.onchange = swap;

Or you could loop over an array of objects and set it there:

var arr = [L40C, L50C];

for(let i = 0; i < arr.length; i++){
  arr[i].onchange = swap;
}

edited Oct 27, 2018 at 8:37

answered Oct 26, 2018 at 17:15

Stramski

1355 bronze badges

3 Comments

StaticFX Over a year ago

if my attempt at the array doesnt work, your 1st example did! thank you!

StaticFX Over a year ago

oo what about something like this? for (i=40; i < 160; i+=10) { L+i+C.onchange=swap; }

Stramski Over a year ago

You could use something like this: for(var i = 0; i < 160; i+=10){document.getElementById("L" + i + "C").onchange = swap;}

Collectives™ on Stack Overflow

Use JS function in place of repeating code

2 Answers 2

1 Comment

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related