Today, I had a pair programming session with my mentor Douglas, which is an awesome experience for me. I learned a lot of advanced programming knowledge from this pair session. And below are some of the notes that deserved to be mentioned:

Early Return

Early return is a coding style that is to help us avoid The IF Spaghetti. Take the following code as an example:

post() {
    const isExternalComponentEvent = [
        'IPostExternalComponentOpened',
        'IPostExternalComponentClosed',
    ].includes(this.bodyParams.event);

    if (isExternalComponentEvent && this.bodyParams.externalComponent)) {
        try {
            const { event } = this.bodyParams;
            const { externalComponent } = this.bodyParams;

            const result = Apps.getBridges().getListenerBridge().externalComponentEvent(event, externalComponent);
        } catch (e) {
            orchestrator.getRocketChatLogger().error(`Error triggering external components' events ${ e.response.data }`);
            return API.v1.internalError();
        }
        return API.v1.success({ result });
    }
    return API.v1.failure({ error: 'Event and externalComponent must be provided.' });
}

Though, there is no problem with the above code, we can use early return to avoid the unnecessary indentation:

post() {
    const isExternalComponentEvent = [
        'IPostExternalComponentOpened',
        'IPostExternalComponentClosed',
    ].includes(this.bodyParams.event);

    if (!this.bodyParams.externalComponent || !isExternalComponentEvent) {
        return API.v1.failure({ error: 'Event and externalComponent must be provided.' });
    } // 👈 early return

    try {
        const { event, externalComponent } = this.bodyParams;
        const result = Apps.getBridges().getListenerBridge().externalComponentEvent(event, externalComponent);

        return API.v1.success({ result });
    } catch (e) {
        orchestrator.getRocketChatLogger().error(`Error triggering external components' events ${ e.response.data }`);
        return API.v1.internalError();
    }
}

With the above refactor, we can reduce the indentations and improve the readability of the code. A few years ago, the JavaScript community was suffering from the indentation issues caused by callback functions - also known as callback hell and we finally solved this problem via the async/await. I think this is also the reason why we prefer the early return instead of wrapping the main logic into the if statement.

Tracker.autorun

Meteor allows developers to use Tracker.autorun to offer a runFunc, so whenever the reactive data sources are changed (such as Session, database queries), the runFunc will be called automatically to update the UI. This is a great feature but when we try to pass an async function as the runFunc, this mechanism will be broken. Let's combine the real code to make a more intuitive understanding 👇

// The code that is not working ☹️
Tracker.autorun(function() {
    APIClient.get('apps/externalComponents').then(({ externalComponents }) => {
        if (!externalComponents.length || !settings.get('Apps_Game_Center_enabled') {
            return TabBar.removeButton('gameCenter');
        }

        TabBar.addButton({
            groups: ['channel', 'group', 'direct'],
            id: 'gameCenter',
            i18nTitle: 'Game_Center',
            icon: 'cube',
            template: 'GameCenter',
            order: -1,
        });
    });
});

// The code that is working 🎉
Tracker.autorun(function() {
    if (!settings.get('Apps_Game_Center_enabled')) {
        return TabBar.removeButton('gameCenter');
    }

    APIClient.get('apps/externalComponents').then(({ externalComponents }) => {
        if (!externalComponents.length) {
            return TabBar.removeButton('gameCenter');
        }

        TabBar.addButton({
            groups: ['channel', 'group', 'direct'],
            id: 'gameCenter',
            i18nTitle: 'Game_Center',
            icon: 'cube',
            template: 'GameCenter',
            order: -1,
        });
    });
});

So what's the difference between them? Douglas made a simulated call stack that can approximately represent what's going on with the above code 😶

Meteor.startup
	Tracker.autorun
		createContext
		gameCenterSetting
			settings.get
				useContext # 👈 used the right context that the Meteor created before
			APIClient.get
promiseResolved
 
Meteor.startup
	Tracker.autorun
		createContext
		gameCenterSetting
			APIClient.get
promiseResolved
	settings.get
		useContext # 👈 lost the context after the promise was resolved 😤

The previous one usessettings.get(...) in an asynchronous function and when it is resolved, it surely will be executed but it will lose the right context. This is possibly the reason why the Meteor Tracker can't track the dependency as expected and run the corresponding function when the sources are changed.

For the next one, we moved the settings.get(...) outside the asynchronous function. We wrote a little more code though, we handled the problem and the UI finally auto-updated as expected 😄

Async/await

The async/await is syntactic sugar of the Promise, which means the following two forms of code are equivalent:

// The Promise way
foo() {
    return new Promise((resolve, reject) => {
        /**
         * Put your asynchronous code here
         */
        resolve(result);
    });
}

// The async/await way
async foo() {
    /**
     * Put your asynchronous code here :)
     * For instance:
     * const a = await API.get('restful.api/a');
     */
    return result; // will return a promise 
}

So for it, no matter whether we need to return a result back to its caller, it's unnecessary for us to add the keyword await before the last the statement inside the async function. The async keyword is only for the previous asynchronous statements.

Lessons

There are also some lessons I learned from this session. Hope I can avoid using them in the future 🙅

Never write unreadable/confusing code

If you can not understand what the code is doing at first glance, then it's surely the unreadable and confusing code 💩. You absolutely need to refactor to make it more readable 👊

Here is a part of code what I wrote before:

if ([8, 46].includes(e.keyCode) && e.target.value === '') {
    const { deleteLastItem } = t;

    return deleteLastItem && deleteLastItem();
}

At first glance, you don't exactly know what these two keycodes 8 and 46 mean (unless you recited all the keycodes before 😶). So we need to add two other constants to make it more clear to others:

const KEYCODE_BACKSPACE = 8;
const KEYCODE_DELETE = 46;

t.ac.onKeyDown(e);
if ([KEYCODE_BACKSPACE, KEYCODE_DELETE].includes(e.keyCode) && e.target.value === '') {
    const { deleteLastItem } = t;
    return deleteLastItem && deleteLastItem();
}

Looks much better 🎉

Type before

Another best practice that I can learn from Douglas is that we can use the type of constants as the prefix to indicate others these constants' types or what the scope these constants belong to.

For example:

// Bad 🙅
const BACKSPACE_KEYCODE = 8;
const DELETE_KEYCODE = 46;

// Good 👍
const KEYCODE_BACKSPACE = 8;
const KEYCODE_DELETE = 46;

This rule can be also applied to *.i18n.json files. For instance:

{
    ...
    "Game_Center": "Game Center",
    "Game_Center_Play_Game_Together": "@here Let's play __name__ together!",
    ...
}

Singleton pattern

In some situations, we apply singleton pattern to save the system resources (usually for some managers). It can also be used to avoid global variables of pollution.

Conclusion

I am so appreciated that I had a wonderful pair programming session with Douglas today and it is really helpful for me to have a better understanding of both our codebase and advanced programming knowledge. Extremely looking forward to the next one!

References

1. Code JS in Style: Early Return
2. Documentation of Tracker, Meteor's reactive system.