3
\$\begingroup\$

I have a JavaScript function to categorize data by their "styles" attribute, and store the key-"unicode" pair.

Here's my current code:

function processMetadata(metadata) {
    let regular = {}, solid = {}, brands = {}
    for (let icon in metadata) {
        let styles = metadata[icon].styles
        let codePoint = metadata[icon].unicode
        if (styles.includes('regular')) {
            regular[icon] = codePoint
        }
        if (styles.includes('solid')) {
            solid[icon] = codePoint
        }
        if (styles.includes('brands')) {
            brands[icon] = codePoint
        }
    }
    return {regular, solid, brands}
}

However, CodeClimate keeps complaining about

Function processMetadata has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

I want to resolve this issue, but I'm not aware of any way to further reduce the complexity without reducing the performance of this code. The following snippet does reduce the cognitive complexity to under 5 but at the expense of performance (3+3 loops instead of 1) and extra dependency:

function processMetadata(metadata) {
    let regular = _.pickBy(metadata, icon => icon.styles.includes('regular'))
    regular = _.mapValues(regular, icon => icon.unicode)
    let solid = _.pickBy(metadata, icon => icon.styles.includes('solid'))
    solid = _.mapValues(solid, icon => icon.unicode)
    let brands = _.pickBy(metadata, icon => icon.styles.includes('brands'))
    brands = _.mapValues(brands, icon => icon.unicode)
    return {regular, solid, brands}
}

Is there any way I can achieve this without having a significant impact on the performance?

\$\endgroup\$
  • \$\begingroup\$ Enforcing a complexity of 5 or lower seems too harsh. Is this a default setting, a personal setting or a team/company setting? \$\endgroup\$ – konijn Jul 18 at 15:41
  • \$\begingroup\$ @konjin It's the default setting. It's not that I have to satisfy the requirement, but I'd just like to know if there is any way to achieve it. \$\endgroup\$ – zypA13510 Jul 19 at 1:01
  • \$\begingroup\$ Could you include an example input and output object? \$\endgroup\$ – morbusg Jul 20 at 18:08
2
\$\begingroup\$

First off, some code review items;

  • Don't skip on semicolons
  • You are not changing a number of variables, consider const over let in those cases
  • The names of your styles match the names of your variables, you could use this

Given that, I would consider something like this

function processMetadata(metadata) {
    let out = {regular: {}, solid: {}, brands: {}};
    for (const icon in metadata) {
        const styles = metadata[icon].styles;
        const codePoint = metadata[icon].unicode;
        for(const style in out){
            if (styles.includes(style)) {
                out[style][icon] = codePoint;
            }
        }
    }
    return out;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ If you make the inner loop an Array.forEach then the callback functions content is not counted in the main functions complexity and should reduce the whole thing to 2 (depending on how you count it) \$\endgroup\$ – Blindman67 Jul 18 at 14:35
  • 1
    \$\begingroup\$ It would have been Object.keys(out).forEach() because out is an Object. I had considered it and didn't go for it in the end. \$\endgroup\$ – konijn Jul 18 at 15:29
  • \$\begingroup\$ I think the semicolon or the lack of one is more of a personal/organizational preference than an issue? If you look at all the style guides over the Internet, some actually enforce no-semicolon. By the way, the project uses ESLint and it will warn you of any potential pitfall when relying on ASI. \$\endgroup\$ – zypA13510 Jul 19 at 1:13
  • \$\begingroup\$ Thank you, this does reduce the cognitive complexity to 6 (still 1 away from 5 🤦‍♂️) at the expense of 14% performance(which is acceptable for me). \$\endgroup\$ – zypA13510 Jul 19 at 2:28
  • \$\begingroup\$ Blindman67's suggestion works to get to 5. As for semicolons, I think it is idiomatic and will become more idiomatic over time. No-semicolons smell like a fad to me. (Very personal opinion of course) \$\endgroup\$ – konijn Jul 19 at 12:14

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.