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.

Sign up for GitHub

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

Jump to bottom

Future #432

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Future #432

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71c1a9b
Implement Future stateless and lazy Promise analogue
tshemsedinov Jun 9, 2019
65ec4dc
Implement futurify callback-last to future-returning
tshemsedinov Jun 10, 2019
7b893dd
Add Future.err() and rename Future.fork to .run
tshemsedinov Jun 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 45 additions & 0 deletions lib/future.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';

class Future {
constructor(executor) {
this.executor = executor;
}

static of(value) {
return new Future(resolve => resolve(value));
}

chain(fn) {
return new Future((resolve, reject) =>
this.fork(
value => fn(value).fork(resolve, reject),
error => reject(error)
)
);
}

map(fn) {
return this.chain(
value =>
tshemsedinov marked this conversation as resolved.
Show resolved Hide resolved
new Future((resolve, reject) => {
try {
resolve(fn(value));
} catch (error) {
reject(error);
}
})
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this perhaps be rewritten without double Future?

  map(fn) {
    return new Future((resolve, reject) => {
      this.run(value => {
        try {
          resolve(fn(value));
        } catch (error) {
          reject(error);
        }
      });
    });

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing the attention here!
I was able to spot the inconsistency of error handling.
The ideologically correct way to implement map function for any monad is the application of chain to the composition of of and fn. Rambda did it.
To do it here one simply needs to move error checking from here to run method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation of map through already existed chain and of is less efficient because of the creation of two Futures. That said such implementation reuses code and is the same for any monad. Though the duplicated code here is tiny meaning it may be allowed here for clarity.
So there is a tradeoff between performance and ideological purity here :-)


fork(successed, failed) {
DzyubSpirit marked this conversation as resolved.
Show resolved Hide resolved
this.executor(successed, failed);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to catch errors the executor may throw. It will also allow omitting error handling in the map method

}

promise() {
return new Promise((resolve, reject) => {
this.fork(value => resolve(value), error => reject(error));
});
}
}

module.exports = { Future };
1 change: 1 addition & 0 deletions metasync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const submodules = [
'control', // Control flow utilities
'do', // Simple chain/do
'fp', // Async utils for functional programming
'future', // Future stateless and lazy Promise analogue
'memoize', // Async memoization
'poolify', // Create pool from factory
'queue', // Concurrent queue
Expand Down
92 changes: 92 additions & 0 deletions test/future.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
'use strict';

const { Future } = require('..');
const metatests = require('metatests');

metatests.test('Future map/fork', async test => {
Future.of(3)
.map(x => x ** 2)
.fork(value => {
test.strictSame(value, 9);
test.end();
});
});

metatests.test('Future lazy', async test => {
Future.of(3).map(test.mustNotCall());
test.end();
});

metatests.test('Future resolve', async test => {
new Future(resolve => {
setTimeout(() => {
resolve(5);
}, 0);
}).fork(value => {
test.strictSame(value, 5);
test.end();
}, test.mustNotCall());
});

metatests.test('Future reject', async test => {
new Future((resolve, reject) => {
reject(new Error('msg'));
}).fork(test.mustNotCall(), error => {
test.strictSame(error.message, 'msg');
test.end();
});
});

metatests.test('Future promise', async test => {
const value = await Future.of(6)
.map(x => ++x)
.map(x => x ** 3)
.promise();

test.strictSame(value, 343);
test.end();
});

metatests.test('Future catch', async test => {
Future.of(6)
.map(() => {
throw new Error('msg');
})
.fork(test.mustNotCall, error => {
test.strictSame(error.message, 'msg');
test.end();
});
});

metatests.test('Future stateless', async test => {
const f1 = Future.of(3);
const f2 = f1.map(x => ++x);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const f2 = f1.map(x => ++x);
const f2 = f1.map(x => x + 1);

const f3 = f2.map(x => x ** 3);
const f4 = f1.map(x => x * 2);

f1.fork(value => {
test.strictSame(value, 3);
});

f1.fork(value => {
test.strictSame(value, 3);
});

f2.fork(value => {
test.strictSame(value, 4);
});

f2.fork(value => {
test.strictSame(value, 4);
});

f3.fork(value => {
test.strictSame(value, 64);
});

f4.fork(value => {
test.strictSame(value, 6);
});

test.end();
});