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

Future #432

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions lib/adapters.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const { Future } = require('./future');

// Convert Promise to callback-last
// promise <Promise>
// callback <Function>
Expand Down Expand Up @@ -67,10 +69,23 @@ const promisifySync = fn => (...args) => {
return Promise.resolve(result);
};

// Convert callback contract to Future-returning function
// fn <Function> callback-last function
//
// Returns: <Function> Future-returning function
const futurify = fn => (...args) =>
new Future((resolve, reject) => {
fn(...args, (err, data) => {
if (err) reject(err);
else resolve(data);
});
});

module.exports = {
callbackify,
asyncify,
promiseToCallbackLast,
promisify,
promisifySync,
futurify,
};
46 changes: 46 additions & 0 deletions lib/future.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

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

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

static err(error) {
return new Future((resolve, reject) => reject(error));
}

chain(fn) {
return new Future((resolve, reject) =>
this.run(value => fn(value).run(resolve, reject), 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.

Suggested change
this.run(value => fn(value).run(resolve, reject), error => reject(error))
this.run(value => fn(value).run(resolve, reject), reject)

Copy link
Member

Choose a reason for hiding this comment

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

ditto

);
}

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 :-)


run(successed, failed) {
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.run(value => resolve(value), 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.

Suggested change
this.run(value => resolve(value), error => reject(error));
this.run(value => resolve(value), reject);

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 the lambda is used for dropping other arguments

});
}
}

module.exports = { Future };
1 change: 1 addition & 0 deletions metasync.js
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
133 changes: 133 additions & 0 deletions test/future.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
'use strict';

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

metatests.test('Future map/run', async test => {
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
metatests.test('Future map/run', async test => {
metatests.test('Future map/run', test => {

?

(and in other tests)

Future.of(3)
.map(x => x ** 2)
.run(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);
}).run(value => {
test.strictSame(value, 5);
test.end();
}, test.mustNotCall());
});

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

metatests.test('Future error', async test => {
Future.err(new Error('msg')).run(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');
})
.run(test.mustNotCall, error => {
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
.run(test.mustNotCall, error => {
.run(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.run(value => {
test.strictSame(value, 3);
});

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

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

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

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

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

test.end();
});

metatests.test('Future futurify success', async test => {
const f1 = (a, b, callback) => {
if (typeof a !== 'number' || typeof b !== 'number') {
callback(new Error('Arguments must be numbers'));
return;
}
callback(null, a + b);
};

const f2 = futurify(f1);

f2(10, 20).run(value => {
test.strictSame(value, 30);
test.end();
});
});

metatests.test('Future futurify fail', async test => {
const f1 = (a, b, callback) => {
if (typeof a !== 'number' || typeof b !== 'number') {
callback(new Error('Arguments must be numbers'));
return;
}
callback(null, a + b);
};

const f2 = futurify(f1);

f2('10', '20').run(test.mustNotCall, error => {
test.strictSame(error.message, 'Arguments must be numbers');
test.end();
});
});