8

Recently I was shown a piece of code that were asked during a full-stack developer interview. It involved creating a Promise, in which the candidate should implement, passing it a resolve function, and chaining 2 then's.

I tried implementing the Promise very naively only to make the code work. Created a ctor that accepts a resolver func, Created a Then function that accepts a callback and returns a Promise, and simply calls the callback on the resolver function.

class MyPromise {

    constructor(resolver) {
        this.resolver = resolver;
    }

    then(callback) {
        const result = new MyPromise(callback);
        this.resolver(callback);

        return result;
    }
}

promise = new MyPromise(
    (result) => {
        setTimeout(result(2), 500);
    });
promise.then(result => {
    console.log(result);
    return 2 * result;
}).then(result => console.log(result));

The expected result is 2,4 - just like operating with real Promise. But i'm getting 2,2. I'm having trouble figuring out how to get the return value for the first "then" and passing it along.

Erez
  • 205
  • 3
  • 13
  • What is `r2`...? And which part of the code is the part that was given to you? – trincot Apr 05 '19 at 13:30
  • 1
    Your concept of a promise is incorrect. Have a look at [Distilling how a promise works](https://stackoverflow.com/a/15668353/1048572) – Bergi Apr 05 '19 at 13:49
  • does the shared code work for you? even the setTimeout accepts a callback. can you please post the code which worked for you and resulted 2,2 – AZ_ Apr 05 '19 at 13:51
  • 1
    @trincot I'm assuming `r2` should be `result` – etarhan Apr 05 '19 at 14:06
  • @etarhan, I understand, but this is really for the OP to tell us. – trincot Apr 05 '19 at 14:24
  • The call to `setTimeout` is wrong. You're executing `result(2)` immediately, and so there is no asynchronous behavior there. This question has issues.... – trincot Apr 05 '19 at 14:53
  • 1
    @AZ_ this is it. just edited the "r2" thingy that sneaked in by mistake – Erez Apr 05 '19 at 16:43

5 Answers5

3

Here's the shortened code for creating a promise class,

class MyPromise {
  constructor(executor) {
    this.callbacks = [];

    const resolve = res => {
      for (const { callback } of this.callbacks) {
        callback(res);
      }
    };

    executor(resolve);
  }

  then(callback) {
    return new MyPromise((resolve) => {
      const done = res => {
        resolve(callback(res));
      };
      this.callbacks.push({ callback: done });
    });
  }
}


promise = new MyPromise((resolve) => {
  setTimeout(() => resolve(2), 1000);
});

promise.then(result => {
  console.log(result);
  return 2 * result;
}).then(result => console.log(result));
AZ_
  • 3,094
  • 1
  • 9
  • 19
  • Why make `callbacks` an array of objects? Just put in the functions themselves – Bergi Apr 05 '19 at 15:23
  • Could you please elaborate on which aspects your implementation is shortened (incomplete) in comparison to a real implementation? – Bergi Apr 05 '19 at 15:24
  • @Bergi surely we could do that, but later promise can be appended to wait for the callback to get resolved and have an another Boolean param say isFullfilled and if not the callback can be pushed into an array until it does not and if fulfilled it directly can call the function. – AZ_ Apr 05 '19 at 15:46
  • No, this functionality should be implemented inside the function, the `callbacks` array should not be concerned with that. – Bergi Apr 05 '19 at 15:54
  • 1
    This does not work when the MyPromise constructor callback calls `resolve` immediately, instead of with `setTimeout`. – trincot Apr 05 '19 at 17:40
2

Your question has some issues:

  • The r2 variable is nowhere defined. I will assume result was intended.
  • The setTimeout is doing nothing useful, since you execute result(2) immediately. I will assume setTimeout(() => result(2), 500) was intended.

If the code was really given like that in the interview, then it would be your job to point out these two issues before doing anything else.

One issue with your attempt is that the promise returned by the then method (i.e. result) is never resolved. You need to resolve it as soon as the this promise is resolved, with the value returned by the then callback.

Also, the promise constructor argument is a function that should be executed immediately.

In the following solution, several simplifications are made compared to a correct Promise behaviour.

  • It does not call the then callbacks asynchronously;
  • It does not support multiple then calls on the same promise;
  • It does not provide the rejection path;
  • It does not prevent a promise from resolving twice with a different value;
  • It does not deal with the special case where a then callback returns a promise

console.log("Wait for it...");

class MyPromise {
    constructor(executor) {
        executor(result => this.resolve(result));
    }
    resolve(value) {
        this.value = value;
        this.broadcast();
    }
    then(onFulfilled) {
        const promise = new MyPromise(() => null);
        this.onFulfilled = onFulfilled;
        this.resolver = (result) => promise.resolve(result);
        this.broadcast();
        return promise;
    }
    broadcast() {
        if (this.onFulfilled && "value" in this) this.resolver(this.onFulfilled(this.value)); 
    }
};

// Code provided by interviewer, including two corrections
promise = new MyPromise(
    (result) => {
        setTimeout(()=>result(2), 500); // don't execute result(2) immediately
    });
promise.then(result => {
    console.log(result); // Changed r2 to result.
    return 2 * result;
}).then(result => console.log(result));

Note the 500ms delay in the output, which is what should be expected from the (corrected) setTimeout code.

I posted a full Promises/A+ compliant promise implementation with comments in this answer

trincot
  • 317,000
  • 35
  • 244
  • 286
2

How about very simple:

const SimplePromise = function(cb) {
  cb(
    data =>
      (this.data = data) &&
      (this.thenCb || []).forEach(chain => (this.data = chain(this.data))),
    error =>
      (this.error = error) &&
      (this.catchCb || []).forEach(chain => (this.error = chain(this.error)))
  );
  this.then = thenCb =>
    (this.thenCb = [...(this.thenCb || []), thenCb]) && this;
  this.catch = catchCb =>
    (this.catchCb = [...(this.catchCb || []), catchCb]) && this;
};

Example Here: https://codesandbox.io/s/0q1qr8mpxn

Manuel Richarz
  • 1,916
  • 13
  • 27
  • /very simple/ kudos for brevity however this is so slim it has very limited utility. No then chaining in particular. – ChrisM Apr 05 '19 at 15:49
  • rather returning this in then you must return the same function with a callback else it wont support chaining; – AZ_ Apr 05 '19 at 15:51
0

Implying that this r2 is actually the result parameter. The problem with your code is that you do not retrieve the result from the result(2). The first "then" gets executed, prints 2, return 4, but this 4 is just wasted. I wrote some code with synchronous function only to demonstrate what to do if you want to get this 2,4 output:

class MyPromise {
    constructor(resolver) {
        this.resolver = resolver;
    }

    then(callback) {
          var res = callback(this.resolver());
        var result = new MyPromise(() => { return res; });

        return result;
    }
}

let promise = new MyPromise(
    () => {
        return 2;
    });

promise
.then(result => {
    console.log(result);
    return 2 * result;
})
.then(result => {
        console.log(result);
});

If you want the resolver to be somewhat async you should use Promises (because return value from function executed inside setTimeout can be retrieved, see here.

If you are not allowed to use these built-in Promises, you can write them yourself with some dummy deferred object and await them (setInterval) to resolve (should be basically the same logic).

antanta
  • 618
  • 8
  • 16
0

There are a few problems with your original code. Notably you are only executing the constructor argument upon call of the then method and you aren't actually chaining the outputs of the 'then' callbacks.

Here is a very (very!) basic promise implementation based upon adapting your example. It will also work for cases where 'then' is called after the promise is resolved (but not if 'then' is already called - multiple then blocks is not supported).

class MyPromise {
    constructor(resolver) {
        let thisPromise = this;

        let resolveFn = function(value){
            thisPromise.value = value; 
            thisPromise.resolved = true;
            if(typeof thisPromise.thenResolve === "function"){
                thisPromise.thenResolve();
            }
            
        }
        if (typeof resolver === "function") {
            resolver(resolveFn);
        }
    }
    then(callback) {
        let thisPromise = this;
        thisPromise.thenFn = callback;

        return new MyPromise((resolve) =>{
            thisPromise.thenResolve = () => {
                thisPromise.value = thisPromise.thenFn(thisPromise.value);
                resolve(thisPromise.value);
            }
            //automatically resolve our intermediary promise if 
            //the parent promise is already resolved
            if(thisPromise.resolved){
                thisPromise.thenResolve();
            }
        });

        
    }
};

//test code

console.log("Waiting for Godot...");

promise = new MyPromise((resolve) =>{
    setTimeout(()=>{
        resolve(2)
    },500);
});


promise.then((result) => {
    console.log(result);
    return 2 * result;
}).then((result) => {
    console.log(result);
    return 2 * result;
}).then((result) => {
    console.log(result)
}); 
ChrisM
  • 1,565
  • 14
  • 24