1

I need to implement an async singleton, that creates a single instance of a class that requires asynchronous operations in order to pass arguments to the constructor. I have the following code:

class AsyncComp {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }

    // A factory method for creating the async instance
    static async createAsyncInstance() {
        const info = await someAsyncFunction();
        return new AsyncComp(info.x, info.y);
    }

    // The singleton method
    static getAsyncCompInstance() {
        if (asyncCompInstance) return asyncCompInstance;
        asyncCompInstance = AsyncComp.createAsyncInstance();
        return asyncCompInstance;
    }
}

The code seems to work fine, as long as the promise fulfils. If, however, the promise is rejected the next calls to getAsyncCompInstance() will return the unfulfilled promise object, which means that it will not be possible to retry the creation of the object. How can I solve this?

omer
  • 1,242
  • 4
  • 18
  • 45
  • I don't see any reason, if the promise is properly rejected, it should work. Can we also check `someAsyncFunction`? – sjahan Jan 06 '20 at 12:28
  • @sjahan even if the promise is rejected, asyncCompInstance is not null anymore, so getAsyncCompInstance() returns it instead of continue to the next line that calls createAsyncInstance() – omer Jan 06 '20 at 13:01
  • if `someAsyncFunction` throws an error the next line will not be executed and hence no instance will be returned. You could put a try catch in static method and return null in case of error. – Ashish Modi Jan 06 '20 at 13:02
  • @AshishModi, this is not accurate. getAsyncCompInstance() does not await createAsyncInstance(), it returns the promise immediately. The code that calls getAsyncCompInstance() takes care of the error handling, and if getAsyncCompInstance() throws/rejects, this code will catch it. – omer Jan 06 '20 at 13:06
  • my bad. totally missed the "does not wait" part. – Ashish Modi Jan 06 '20 at 13:11

1 Answers1

1

So after thinking about a couple of possible solution, I decided to wrap the asynchronous call in createAsyncInstance() with a try/catch block, and if it failed, set asyncCompInstance back to be null, and throw the error the occurred. This way, if the object creation failed, the caller can call getAsyncCompInstance() again to try to get an instance of the class:

class AsyncComp {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }

    // A factory method for creating the async instance
    static async createAsyncInstance() {
        try {
            const info = await someAsyncFunction();
            return new AsyncComp(info.x, info.y);
        }
        catch (err) {
            asyncCompInstance = null;
            throw err;
        }
    }

    // The singleton method
    static getAsyncCompInstance() {
        if (asyncCompInstance) return asyncCompInstance;
        asyncCompInstance = AsyncComp.createAsyncInstance();
        return asyncCompInstance;
    }
}

I know this is not the cleanest code, and specifically not the classic factory pattern implementation, but this is the best I could come up with at the moment. Would love to hear comments/suggestions on this solution.

omer
  • 1,242
  • 4
  • 18
  • 45
  • 2
    The approach is fine (similar to [what I would do](https://stackoverflow.com/a/43018394/1048572)), but for cleanliness I'd move the `try`/`catch` into the `getAsyncCompInstance` method whose job it is to store the singleton. Or really just merge them into a single method, as you never would want to call `createAsyncInstance` individually (ignoring the singleton). – Bergi Jan 06 '20 at 22:51
  • 2
    A further refactor might move the `new AsyncComp` call outside of the `try` block - you would only want to handle errors from `someAsyncFunction`, but retrying because the constructor failed might be pointless. Continuing on that, you could just move the retry logic inside of the `someAsyncFunction`, and keep an eventual rejection as the final value. But this depends a lot on your usage scenario, if there are multiple call sites of `getAsyncCompInstance()` and you think it is not flexible enough to share their error handling then you might better avoid a singleton altogether. – Bergi Jan 06 '20 at 22:57