0

I have some code like so:

export async function handleRefresh() {
    if (!existsSync('postr.toml')) fail('not a postr directory');

    const posts = expandGlob('posts/*');

    for await (const post of posts) {
        if (!post.isDirectory) {
            console.warn('warning: non-folder found in posts directory');
            continue;
        }

        let {parsedFrontMatter, contents} = extractFrontMatter(await read(post.path + '/post.md'));
        const adapters = parsedFrontMatter.adapters ?? [];

        if (!parsedFrontMatter) {
            fail('no frontmatter for ' + post.path);
            continue;
        }

        if (!Array.isArray(adapters)) {
            fail('adapters is not an array');
            continue;
        }

        if (isValidFrontMatter(parsedFrontMatter)) {
            fail('frontmatter is not valid');
            continue;
        }

        adapters.forEach(async (adapter: string) => {
            const adapterPlugins = parseToml(await read('postr.toml')).adapterPlugins ?? {};

            if (!isObject(adapterPlugins)) {
                fail('adapterPlugins in the configuration is not an object');
                return;
            }

            const adapterPath = adapterPlugins[adapter];

            if (!adapterPath) {
                console.warn('warn: an adapter was set' +
                    'but the corresponding plugin was not configured in `postr.toml`. Skipping');
                return;
            }

            if (!('path' in <any>adapterPath)) {
                fail(`adapter ${adapter} does not have a path`);
                return;
            }

            import((<any>adapterPath).path)
                .then(async module => {
                    const action = getActionForPost(parsedFrontMatter);

                    if (module[action]) {
                        await module[action](contents, parsedFrontMatter, (<any>adapterPath).config, {
                            updateFrontMatter(newData: {[x: string]: any}) {
                                parsedFrontMatter = Object.assign(parsedFrontMatter, newData);
                            },
                            mapID(remote: string | number) {
                                addMapping(parsedFrontMatter.id as string, remote.toString(), adapter);
                            }
                        })
                    } else {
                        console.warn(`Adapter ${adapter} does not support action \`${action}\``);
                        return;
                    }

                    writeFinalContents(parsedFrontMatter, contents, post.path)
                })
                .catch(error => fail(`could not run adapter because of ${error.name}: ${error.message}`));

        });
    }
}

Huge function.

There are a lot of these necessary if checks. 3/4 of the function is if checks, you could say. I want some advice on how I could refactor these statements.

As you can see the checks are not always the same, there are some different checks going on there.

EDIT: I've added real code.

  • 2
    refactor. why? to what? – Nina Scholz Sep 23 '21 at 09:37
  • 4
    If it is necessary, and it works, then the question is off topic here. – trincot Sep 23 '21 at 09:37
  • @NinaScholz since the if checks take soo much space it's getting harder to understand the function. As for to what, I want some way to move them out of the main function/simplify/etc. – Siddharth Shyniben Sep 23 '21 at 09:39
  • @trincot the checks are necessary, but the code doesn't necessarily have to be written in this way. So still on topic. – Siddharth Shyniben Sep 23 '21 at 09:39
  • 5
    For review of working code, the site [Code Review](https://codereview.stackexchange.com/) is more appropriate, however the question should be completely reformatted to align with the rules over there. – trincot Sep 23 '21 at 09:41
  • you should use switch cases [how to do](https://www.w3schools.com/js/js_switch.asp) – Tsirsuna Sep 23 '21 at 09:43
  • @SirTristanus the conditions are completely different; Moreover using a switch will not make any huge differences except the syntax will be different. – Siddharth Shyniben Sep 23 '21 at 09:44
  • 1
    @Siddharth switch cases are easier to read than a big if forest but yeah you're right you can't use them everywhere but it's still a good alternative – Tsirsuna Sep 23 '21 at 09:46
  • @trincot Thanks for the tip, next time I'll definitely check it out. – Siddharth Shyniben Sep 23 '21 at 09:46
  • 1
    I guess it's a big function. To refactor it, you will need to understand the logic then create/abstract those `if` to smaller functions. With such a pseudo code, I can't give any other better advice – BraveVN Sep 23 '21 at 09:51
  • @BraveVN I've updated with some of the real code to give you an idea of how it is. – Siddharth Shyniben Sep 23 '21 at 09:59

0 Answers0