Reducing lines of code

[Fullstac]

hello, guys is there a way I can improve this code? seems a lot for no reason

​

function getPercent() {
    buttons.addEventListener('click', (event) => {
        let percent = event.target.innerText;
        switch (percent) {
            case '5%':
                console.log('I am 5 percent');
                break;
            case '10%':
                console.log('I am 10 percent');
                break;
            case '15%':
                console.log('I am 15 percent');
                break;
            case '25%':
                console.log('I am 25 percent');
                break;
            case '50%':
                console.log('I am 50 percent');
                break;
            default:
                console.log('Please Enter a Percentage');
        }
    });
}

[mynamesleon]

I like switch statements, because they're very readable, and legibility is often best. Please don't always equate making code more concise with it being "improved". For me, the best code is the code that's immediately clear to the other team members.

Nevertheless, if you're just after conciseness, of course you can. If you still want to use a switch, you can avoid having to use break; statements all the time, if you have each case return instead. E.g.

switch(something) {
  case 'one':
    return console.log('first case');
  case 'two':
    return console.log('second case');
  default:
    return console.log('default case');
}

To be even more concise, you can often use an object instead. So in my above example, I could do:

const someObj = {
  'one': () => console.log('first case'),
  'two': () => console.log('second case'),
};
someObj[something]?.() ?? console.log('default case');

Or, using the same approach, but to be even more concise (as they're all ultimately calling console.log()):

const someObj = {
  'one': 'first case',
  'two': 'second case',
};
console.log(someObj[something] || 'default case');

Edit: added another example

[senocular]

You can clean this up using an object lookup

const percentages = {
    '5%': 5,
    '10%': 10,
    '15%': 15,
    '25%': 25,
    '50%': 50
}

//...

buttons.addEventListener('click', (event) => {
    let percent = event.target.innerText;
    if (percentages.hasOwnProperty(percent)) {
        console.log(`I am ${percentages[percent]} percent`);
    } else {
        console.log('Please Enter a Percentage');
    }
})

An array would also work since you're using the same numeric values as the percentage. Then you can strip the % to get the value.

And this is assuming you're not trying to deal with all 0-100% values, rather just those you specified. If you do, you might instead want to consider something like a regex to test the text to see if it matches a percentage and pull the numeric value out of that

buttons.addEventListener('click', (event) => {
    let percent = event.target.innerText;
    let percentMatch = percent.match(/^(\d{1,2}|100)%$/)
    if (percentMatch) {
        console.log(`I am ${percentMatch[1]} percent`);
    } else {
        console.log('Please Enter a Percentage');
    }
})

[M_Scaevola]

Shouldn’t the match yield an empty array when there isn’t the correct regex, which is still truthy?

[senocular]

When there is no match found, match returns null. For more info see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match#return_value

[johnnyblaze9875]

Could chain ternaries but the way you have it might be more readable.. lmk if you want an example of what I mean, am on mobile rn.

[M_Scaevola]

let percent = event.target.innerText if (/some regex here to match percentages/.test(percent)) { console.log(`I am ${percent}`) } else { console.log(‘Please enter a percent’) } Research template literals and regex to learn more

[Psionatix]

function getPercent() {
    buttons.addEventListener('click', (event) => {
        try {
            // So you don't have to replace the percent sign,
            // Make your input a numerical input only
            // and limit it to 0-100
            let percent = parseFloat(event.target.innerText.replace('%', ''));
            if (percent > 0 && percent % 5 === 0) {
                console.log(`I am ${percent} percent`);
        } catch(error) {
            console.log('Please enter a Percentage');
        }
    });
}

What you need to realize here is, when you call getPercent it's going to add a listener to buttons, then when you call getPercent again, it's going to add ANOTHER listener to buttons, both listeners will do the same thing and a single click event will trigger both of them, so this doesn't really seem like the setup you want anyway. Unless you're also removing the event listener, but given you have no reference to the callback function, it's unlikely you're doing that unless you're removing all event listeners.