-
-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor expectMap/flatMap/map into one function #13
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
base: master
Are you sure you want to change the base?
Changes from all commits
c6c9192
debb189
eee6f9a
1e841aa
3aebf17
5a1485a
c1db9c7
213d571
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| }, | ||
| "dependencies": { | ||
| "helpbox": "^7.2.0", | ||
| "lodash.cond": "^4.5.2", | ||
| "lodash.flow": "^3.5.0" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ const throwArgument = require('./throw-argument'); | |
| const returnThis = require('./return-this'); | ||
| const Exception = require('./exception'); | ||
| const Optional = require('./optional'); | ||
| const cond = require('lodash.cond'); | ||
|
|
||
| const { identity, constant, pipe, property } = require('./utils'); | ||
|
|
||
|
|
@@ -81,10 +82,6 @@ Object.assign(Ok.prototype, { | |
| return this.value === value; | ||
| }, | ||
|
|
||
| expectMap(λ) { | ||
| return Result.expect(λ(this.value)); | ||
| }, | ||
|
|
||
| getOrElse(_) { | ||
| return this.value; | ||
| }, | ||
|
|
@@ -101,12 +98,12 @@ Object.assign(Ok.prototype, { | |
| return this.value; | ||
| }, | ||
|
|
||
| map(λ) { | ||
| return transformResult(() => λ(this.value), Ok); | ||
| }, | ||
|
|
||
| flatMap(λ) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's why I chose to have aliases
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return transformResult(() => λ(this.value), Result.expect); | ||
| transform(λ) { | ||
| try { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DecideWrapper should encapsulate all of the logic to transform values. I see why you need that, but it's a bit weird to split the logic in two places (the try catch and decideWrapper)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point |
||
| return Result.expect(λ(this.value)); | ||
| } catch (error) { | ||
| return Result.Aborted(error); | ||
| } | ||
| }, | ||
|
|
||
| reject(predicate) { | ||
|
|
@@ -155,14 +152,12 @@ Error = function createError(error) { | |
| Error.prototype = Object.create(Result.prototype); | ||
|
|
||
| Object.assign(Error.prototype, { | ||
| map: returnThis.unary, | ||
| tap: returnThis.unary, | ||
| transform: returnThis.unary, | ||
| filter: returnThis.unary, | ||
| reject: returnThis.unary, | ||
| flatMap: returnThis.unary, | ||
| replace: returnThis.unary, | ||
| property: returnThis.unary, | ||
| expectMap: returnThis.unary, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good riddance! |
||
| expectProperty: returnThis.unary, | ||
|
|
||
| get() { | ||
|
|
@@ -183,7 +178,7 @@ Object.assign(Error.prototype, { | |
|
|
||
| recoverWhen(predicate, λ) { | ||
| return Ok(this.error) | ||
| .map(λ) | ||
| .transform(λ) | ||
| .filter(predicate); | ||
| }, | ||
|
|
||
|
|
@@ -246,15 +241,13 @@ Aborted.prototype = Object.create(Result.prototype); | |
|
|
||
| Object.assign(Aborted.prototype, { | ||
| toOptional: None, | ||
| map: returnThis.unary, | ||
| tap: returnThis.unary, | ||
| transform: returnThis.unary, | ||
| filter: returnThis.unary, | ||
| reject: returnThis.unary, | ||
| recover: returnThis.unary, | ||
| flatMap: returnThis.unary, | ||
| mapError: returnThis.unary, | ||
| property: returnThis.unary, | ||
| expectMap: returnThis.unary, | ||
| replace: returnThis.unary, | ||
| expectProperty: returnThis.unary, | ||
| abortOnErrorWith: returnThis.unary, | ||
|
|
@@ -326,16 +319,14 @@ const callWrappedResultMethod = methodName => { | |
|
|
||
| Object.assign(Pending.prototype, { | ||
| asynchronous: returnThis.nullary, | ||
| map: callWrappedResultMethod('map'), | ||
| tap: callWrappedResultMethod('tap'), | ||
| transform: callWrappedResultMethod('transform'), | ||
| filter: callWrappedResultMethod('filter'), | ||
| reject: callWrappedResultMethod('reject'), | ||
| recover: callWrappedResultMethod('recover'), | ||
| replace: callWrappedResultMethod('replace'), | ||
| flatMap: callWrappedResultMethod('flatMap'), | ||
| mapError: callWrappedResultMethod('mapError'), | ||
| property: callWrappedResultMethod('property'), | ||
| expectMap: callWrappedResultMethod('expectMap'), | ||
| recoverWhen: callWrappedResultMethod('recoverWhen'), | ||
| abortOnError: callWrappedResultMethod('abortOnError'), | ||
| expectProperty: callWrappedResultMethod('expectProperty'), | ||
|
|
@@ -375,33 +366,14 @@ const fromPromise = promise => { | |
| return Pending(promise.then(Ok, Error)); | ||
| }; | ||
|
|
||
| const expect = optionalOrResultOrPromise => { | ||
| if ([null, undefined].includes(optionalOrResultOrPromise)) { | ||
| return Error(); | ||
| } else if (typeof optionalOrResultOrPromise !== 'object') { | ||
| return Ok(optionalOrResultOrPromise); | ||
| } | ||
|
|
||
| if (isPromise(optionalOrResultOrPromise)) { | ||
| return Pending(optionalOrResultOrPromise.then( | ||
| Result.expect, | ||
| Result.Aborted | ||
| )); | ||
| } | ||
|
|
||
| if (optionalOrResultOrPromise.isResultInstance) { | ||
| return optionalOrResultOrPromise; | ||
| } | ||
|
|
||
| if (!optionalOrResultOrPromise.isOptionalInstance) { | ||
| optionalOrResultOrPromise = Optional.fromNullable(optionalOrResultOrPromise); | ||
| } | ||
|
|
||
| return optionalOrResultOrPromise.match({ | ||
| Some: Ok, | ||
| None: Error | ||
| }); | ||
| }; | ||
| const expect = cond([ | ||
| [value => [null, undefined].includes(value), constant(Error())], | ||
| [value => typeof value !== 'object', value => Ok(value)], | ||
| [value => isPromise(value), value => Result.Pending(value.then(Result.expect, Aborted))], | ||
| [value => value.isResultInstance, value => value], | ||
| [value => value.isOptionalInstance, value => value.match({ Some: Ok, None: Error })], | ||
| [constant(true), value => Optional.fromNullable(value).match({ Some: Ok, None: Error })] | ||
| ]); | ||
|
|
||
| const when = (truthy, value, error) => { | ||
| if (truthy) { | ||
|
|
@@ -411,7 +383,11 @@ const when = (truthy, value, error) => { | |
| return Error(error); | ||
| }; | ||
|
|
||
| [['transform', 'map']].forEach(([alias, method]) => { | ||
| [ | ||
| ['bind', 'transform'], | ||
| ['chain', 'transform'], | ||
| ['flatMap', 'transform'], | ||
| ].forEach(([alias, method]) => { | ||
| Ok.prototype[alias] = Ok.prototype[method]; | ||
| Error.prototype[alias] = Error.prototype[method]; | ||
| Pending.prototype[alias] = Pending.prototype[method]; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| /* global describe, it */ | ||
|
|
||
| const { expect } = require('chai'); | ||
| const Result = require('../source/result'); | ||
| const { constant, identity } = require('../source/utils'); | ||
|
|
||
| const { Error, Aborted } = Result; | ||
|
|
||
| const increment = n => n + 1; | ||
|
|
||
| process.on('unhandledRejection', (reason, promise) => { | ||
| console.log(reason); | ||
| }); | ||
|
|
||
| describe('The Aborted subtype', () => { | ||
| it('should have the proper flags', () => { | ||
| const error = Aborted(); | ||
|
|
||
| expect(error.isResultInstance).to.equal(true); | ||
| expect(error.isAsynchronous).to.equal(false); | ||
| expect(error.isAborted).to.equal(true); | ||
| expect(error.isError).to.equal(true); | ||
| expect(error.isOk).to.equal(false); | ||
| }); | ||
|
|
||
| it('should be an error that you can only unwrap at the end of a computation transform', () => { | ||
| const value = 3; | ||
|
|
||
| const aborted = Aborted(value) | ||
| .transform(increment) | ||
| .abortOnError() | ||
| .replace(Promise.resolve(5)) | ||
| .abortOnErrorWith(increment) | ||
| .transform(increment) | ||
| .recover(increment) | ||
| .property('a') | ||
| .tap(increment) | ||
| .expectProperty('a') | ||
| .reject(constant(true)) | ||
| .recoverWhen(increment, increment) | ||
| .transform(increment) | ||
| .mapError(increment); | ||
|
|
||
| expect(aborted.isError).to.equal(true); | ||
| expect(aborted.merge()).to.equal(value); | ||
| expect(aborted.isAborted).to.equal(true); | ||
| expect(aborted.valueEquals(value)).to.equal(false); | ||
| expect(aborted.satisfies(() => true)).to.equal(false); | ||
| expect(aborted.satisfies(() => false)).to.equal(false); | ||
| }); | ||
|
|
||
| describe('get()', () => { | ||
| const value = 3; | ||
|
|
||
| try { | ||
| Aborted(value).get(); | ||
|
|
||
| expect(false).to.equal(true); | ||
| } catch (error) { | ||
| expect(error).to.equal(value); | ||
| } | ||
| }); | ||
|
|
||
| describe('getOrElse(value)', () => { | ||
| it('should return `value`', () => { | ||
| const value = 3; | ||
|
|
||
| expect(Aborted(value + 1).getOrElse(value)).to.equal(value); | ||
| }); | ||
| }); | ||
|
|
||
| describe('match(callbacks)', () => { | ||
| it('should execute callbacks.Aborted if it exists', () => { | ||
| const expected = 3; | ||
|
|
||
| expect( | ||
| Aborted(3).match({ | ||
| Aborted: constant(expected), | ||
| Error: constant(increment(expected)) | ||
| }) | ||
| ).to.equal(expected); | ||
| }); | ||
|
|
||
| it('should just throw the wrapped error when the callbacks.Aborted callback does not exist', () => { | ||
| const value = 3; | ||
|
|
||
| try { | ||
| Aborted(value).match({}); | ||
|
|
||
| expect(false).to.equal(true); | ||
| } catch (error) { | ||
| expect(error).to.equal(value); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('toOptional()', () => { | ||
| it('should transform the Aborted into a None', () => { | ||
| const expected = 3; | ||
|
|
||
| const optional = Error(expected).toOptional(); | ||
|
|
||
| expect(optional.isOptionalInstance).to.equal(true); | ||
| expect(optional.valueAbsent).to.equal(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('toPromise()', () => { | ||
| it('should transform the Aborted into a rejeced promise', done => { | ||
| const expected = 3; | ||
|
|
||
| const promise = Aborted(expected).toPromise(); | ||
|
|
||
| expect(promise.then !== undefined).to.equal(true); | ||
|
|
||
| promise | ||
| .then(constant(increment(expected))) | ||
| .catch(identity) | ||
| .then(value => { | ||
| expect(value).to.equal(expected); | ||
| }) | ||
| .then(done); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to preserve compat for this which users of a monad-like will expect there to exist, so we can add but not remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, I like to have something that is clear and predictable instead of something magical (which more often than not is unpredictable!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I thought map is redundant in monads. flatMap already does that
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mapandflatMapare supposed to be different in monads.For any monad
Monad:map(orfmap) takes a function which has the signature:a => bwhereas
flatMap (or
bind,chain, or(>>=)) takes a function with the signaturea => Monad b