2

Here is my closure that creates a promise that resolves to an object:

function createConnection(host, port, timeout) {
    let latestTransactionId = 0;
    let commandResolvers = new Map();
    const socket = new WebSocket(`${host}:${port}`);

    function newTransactionId() {
        latestTransactionId += 1;
        return latestTransactionId.toString();
    }

    function request(body) {
        return new Promise(function(resolve, reject) {
            const transactionId = newTransactionId();
            commandResolvers.set(transactionId, resolve);

            socket.send(JSON.stringify({
                ...body,
                trans_id: transactionId
            }));

            setTimeout(function () {
                reject(Error("Command timed out..."));
            }, timeout);    
        });
    }

    return new Promise(function(resolve, reject) {
        socket.onopen = function () {
            socket.onmessage = function(event) {
                const data = JSON.parse(event.data);
                const transactionId = data.trans_id;
                const commandResolver = commandResolvers.get(transactionId)
                commandResolver(data);
            };

            connection = {
                request: request
            }

            resolve(connection);
        };

        setTimeout(function () {
            reject(Error("Connection timed out..."));
        }, timeout);
    });
}

Here is how I use it:

const connection = await createConnection(host, port, timeout);

How would I rewrite the object creation implementation to make this syntax possible:

const connection = await new Connection(host, port, timeout);

or maybe the following would be better

const connection = await new ConnectionPromise(host, port, timeout);

And would that make sense? I still have some problem understanding when I should use a closure to create an object and when I should use a prototype. People also seem to recommend different patterns with very little practical advantages and disadvantages, but I just like the prototype pattern more so would prefer to use it here it it makes sense to do so.

user1283776
  • 19,640
  • 49
  • 136
  • 276
  • It makes sense if you have multiple sockets to listen for. Do you have issues to create the constructor and prototype or is that not part of the question? Concerning the closures, it's not a choice between closures and prototypes. You can mix and match them as desired, but from the way you word it I think you might mean a singleton? Like an object that can only create one instance? Since to put it simple, a closure is just a wrapper around the function and its data while it's in use. – Shilly Nov 23 '18 at 15:25
  • So in your code, calling createConnection() will create a closure around the function createConnection and the data behind the variables socket, resolvers, etc so they can still be accessed after createFunction() finished running. – Shilly Nov 23 '18 at 15:26
  • You could try putting socket, resolvers and id on the `{ request: request }` object, and make `request` (and also `newTransactionId`) a method. Then go to create that object with a constructor and give it prototype methods. But no, that should not change the nature of `createConnection` which should stay a plain function (or at most a static method) that returns a promise for an (instance) object. – Bergi Nov 23 '18 at 17:38

1 Answers1

0

You could make your constructor return a promise,..

eg.

class Connection {
  constructor () {    
    this.name = "connection";
    return new Promise((resolve) => {
      setTimeout(() => {
        this.name = "connection made";
        resolve(this);
      }, 1000);
    });
  }
}

async function test () {
  console.log("Start");
  const a = await new Connection();
  console.log(a.name);
  console.log("End");
}

test();
Keith
  • 22,005
  • 2
  • 27
  • 44
  • Thanks! I'll try that on Monday – user1283776 Nov 23 '18 at 15:34
  • [No, don't do that!](https://stackoverflow.com/questions/24398699/is-it-bad-practice-to-have-a-constructor-function-return-a-promise) – Bergi Nov 23 '18 at 17:35
  • @Bergi Yes, I'm sure it's not a good idea. But out of curiosity how does it mess with inheritance,. Doing instanceof for me returns true, not false as you mention in the link, I also used `extends` to create a sub class, and Connection & Connection2 both returned true for instanceof. Also doing methods & overrides seems to work. This is using Chrome, so I wonder if it's more an ES5 inheritance problem?. The thing I could find is yes, `super` would return a promise, but then wouldn't that be expected? methods `this` are still the instance. – Keith Nov 23 '18 at 22:23
  • Also, just compiled the same test to ES5 using Babel, and again instanceof is returning true.. :( I'm sure I'm missing something, as from past experience I know you know your stuff.. :) Are you able to show a simple example that shows things going pear shaped. – Keith Nov 23 '18 at 22:40
  • ps. Also people remember what the down vote button is for (hint, mouse over and it tells you), it's not to mark an answer as correct / or wrong. Now from my point of view, wrong answers can also be useful, especially if the reason is later explained why. After all we learn from mistakes. – Keith Nov 23 '18 at 22:56
  • @Keith Yes, when you `resolve` with `this` you get a promise for an instance with the correct inheritance hierarchy. But still it's a promise, and `(new Example) instanceof Example` doesn't hold as expected - you would need `(await new Example) instanceof Example`. – Bergi Nov 24 '18 at 09:49
  • 1
    @Keith I recommend `class Connection { constructor(name) { this.name = name; } static create() { return new Promise(resolve => setTimeout(resolve, 1000, "connection made")).then(name => new Connection(name)); } }` – Bergi Nov 24 '18 at 09:51
  • @Bergi: It sounds like you know your stuff. Feel free to post an answer with your recommended solution (ES5 and optionally ES6) and why other solutions are inferior. I have read your comments, but think that I and probably also others would learn from your thoughts on different approaches. EDIT: I see now that you marked the question as duplicate. I'll check out the other thread. – user1283776 Nov 26 '18 at 08:16