Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

review comments #1

Open
phoddie opened this issue Jun 10, 2021 · 0 comments
Open

review comments #1

phoddie opened this issue Jun 10, 2021 · 0 comments
Assignees

Comments

@phoddie
Copy link

phoddie commented Jun 10, 2021

The reversion below suggests a few changes to present pubic interface free of any internal functions or state. There is no significant change in public API or behavior intended.

In addition, the implementation ensures that a given function is only registered once for a given event. When a function is removed, if the listener count for the event goes to zero, the event is removed. That simplifies the code in the rest of the implementation.

The exception on emit when the event is not present is replaced by returning false which seems consistent with the other functions.

When compiled by the Moddable SDK, the private fields in TypeScript do not map to JavaScript fields directly. That adds overhead. It seems like it must be possible to have TypeScript generate standard JavaScript private fields. Suggestions?

interface IEventEmitter {
    on: (event: string, listener: Function) => Boolean | null | undefined,
    emit: (event:string, args: any) => Error | Boolean,
    addListener: (event: string, listener: Function) => Boolean | null | undefined,
    removeListener: (event:string, listener: Function) => Boolean | null | undefined
}

export default class EventEmitter implements IEventEmitter {
    #events: Map<string, Function[] | null | undefined> = new Map()
    #maxListeners: Number;
    constructor(options?: any) {
        this.#maxListeners = options?.maxListeners ?? 10;
    }

    on(event: string, listener: Function) {
        return this.addListener(event, listener);
    }

    emit(event: string, args: any) {
        const listeners = this.#events.get(event);
        if (!listeners) return false;
        listeners.forEach(triggerCallback(args))
        return true;
    }

    addListener(event: string, listener: Function) {
        const listeners = this.#events.get(event);

        if (!listeners) {
            this.#events.set(event, [listener]);
        } else {
            if ((listeners.length >= this.#maxListeners) || listeners.includes(listener))
                return false;

            listeners.push(listener)
        }
        return true;
    }

    removeListener(event: string, listener: Function) {
        const listeners = this.#events.get(event);
        if (!listeners)
            return false;

        const index = listeners.indexOf(listener);
        if (index < 0)
            return false;

        listeners.splice(index, 1);
        if (!listeners.length)
            this.#events.delete(event);

        return true;
    }
}

function triggerCallback(args: any) {
    return (cb: Function) => {
        try {
            cb(args)
        } catch(err) {
            throw new Error(err)
        }
    }
}
@dashcraft dashcraft self-assigned this Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants