Execute function with given parameters

Question

I was wondering if someone could help me refine this piece of code, in my eyes it is not a nice function to execute are there ways to reduce this function in lines?

This is my function:

function changeStyling(input, changeSelector, elementCssChange, pixels) {
   if(!reload){
    if(changeSelector === '.exit-intent-wrapper' && elementCssChange === 'background') {
        this.base64PopupBGImg = null, document.querySelector(changeSelector).style[elementCssChange] = `${input.value}`;
    }

    pixels ? 
        document.querySelector(changeSelector).style[elementCssChange] = `${input.value}px` : 
        document.querySelector(changeSelector).style[elementCssChange] = `${input.value}`;
  } else {
    pixels[i] ? document.querySelector(changeSelector[i]).style[elementCssChange[i]] = `${input[i].value}px` : document.querySelector(changeSelector[i]).style[elementCssChange[i]] = input[i].value;
    for (i = 0; i < arguments.length; i++) {
    }
}

}

And this is how i execute it on reload:

changeStyling(
        [
            document.querySelector(".exitIntentWidth"), 
            document.querySelector(".exitIntentHeight"), 
            document.querySelector(".exitIntentRadius"),
            document.querySelector(".exitIntentPaddingAll"),
            document.querySelector(".exitIntentPadding"),
            document.querySelector(".exitIntentPaddingLeft"),
            document.querySelector(".exitIntentPaddingRight"),

            document.querySelector(".btnWidth"),
            document.querySelector(".btnHeight"),
            document.querySelector(".btnRadius"),
            document.querySelector(".btnColor"),
            document.querySelector(".btnTextColor")
        ], 
        [
            '.exit-intent-wrapper',
            '.exit-intent-wrapper',
            '.exit-intent-wrapper',
            '.exit-intent-wrapper',
            '.exit-intent-inner',
            '.exit-intent-inner',
            '.exit-intent-inner',

            '.button',
            '.button',
            '.button',
            '.button',
            '.button'
        ],
        [
            'width',
            'height',
            'borderRadius',
            'padding',
            'padding',
            'paddingLeft',
            'paddingRight',

            'width',
            'height',
            'borderRadius',
            'backgroundColor',
            'color'
        ],
        [ true, true, true, true, true, true, true, true, true, true, false, false ]
    );

Does someone know how I can reduce the execute of the function?

If it's working code it likely belongs on the code review stack exchange. — Dave Newton
– Dave Newton, Commented Mar 10, 2018 at 20:51
Don't use ?: in void context. foo ? x = bar : x = baz; is better written as x = foo ? bar : baz;. — melpomene
– melpomene, Commented Mar 10, 2018 at 20:51
you'd have to at least tell us the exact requirements, otherwise we don't know which bits are potentially redundant — ADyson
– ADyson, Commented Mar 10, 2018 at 20:51
But seriously, you're way overusing ?:. Half of those should be if. — melpomene
– melpomene, Commented Mar 10, 2018 at 20:52
An array of objects is better than a bunch of parallel arrays (i.e. use objects[i].foo and objects[i].bar instead of foo[i] and bar[i]). — melpomene
– melpomene, Commented Mar 10, 2018 at 20:54

Ben · Accepted Answer · 2018-03-10 21:07:37Z

Your code currently uses parallel arrays, which are very confusing and difficult to modify. I recommend either multidimensional arrays or objects.

function changeStyling(input) {
   if(!reload){
    if(input.changeSelector === '.exit-intent-wrapper' && input.elementCssChange === 'background') {
        this.base64PopupBGImg = null, document.querySelector(input.changeSelector).style[input.elementCssChange] = `${input.el.value}`;
    }

    pixels ? 
        document.querySelector(changeSelector).style[input.elementCssChange] = `${input.el.value}px` : 
        document.querySelector(changeSelector).style[input.elementCssChange] = `${input.el.value}`;
  } else {
    pixels[i] ? document.querySelector(input.changeSelector[i]).style[input.elementCssChange[i]] = `${input.el[i].value}px` : document.querySelector(input.changeSelector[i]).style[input.elementCssChange[i]] = input.el[i].value;
    for (i = 0; i < arguments.length; i++) {
    }
}


changeStyling(
        [
            {
                el:document.querySelector(".exitIntentWidth"),
                changeSelector:'.exit-intent-wrapper',
                elementCssChange:'width'
                //Continue for any necessary modifications
            },
            {
                el:document.querySelector(".exitIntentWidth"),
                changeSelector:'.exit-intent-wrapper',
                elementCssChange:'width'
                //Continue for any necessary modifications
            },
        ]);

Please note, I haven't tested this code, it might not work like expected

Collectives™ on Stack Overflow

Execute function with given parameters

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related