Skip to content

Implemented following two array helper functions with corresponding unit tests #133

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

Closed
wants to merge 1 commit into from
Closed

Implemented following two array helper functions with corresponding unit tests #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Result/ResultType.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,29 @@ infix operator &&& {
public func &&& <L: ResultType, R: ResultType where L.Error == R.Error> (left: L, @autoclosure right: () -> R) -> Result<(L.Value, R.Value), L.Error> {
return left.flatMap { left in right().map { right in (left, right) } }
}

/// Return a array of `V` by applying transform to all elements of array 'U' and filtering out failure ones
public func mapFilter<U, V, Error>(results: [U], @noescape transform: U -> Result<V, Error>) -> [V]{
Copy link
Contributor

Choose a reason for hiding this comment

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

If expressed as an extension method on CollectionType, I think this becomes essentially equivalent to a straightforward use of the collection-to-optional variant of flatMap in the standard library:

values.mapFilter(transform)
// is equivalent to
values.flatMap { transform($0).value }

In which case maybe we’d be better to just use the standard library’s functionality and not extend it with this particular method.

Copy link
Author

Choose a reason for hiding this comment

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

Great response, @robrix !
And I agree that a particular mapFilter is not necessary here.

BTW: my first version of `mapFilter' is something like 'values.lazy.map(transform).filter(...).map{$0!}' which is far more complicated than the version your provided.
:+1

var vs = [V]()
for result in results{
if case let .Success(value) = transform(result){
vs.append(value)
}
}
return vs
}

/// Return a Result with an array of `V`s if all Results of applying `transform` are `Success`es or return the error of the first `Failure`
public func mapM<U, V, Error>(results: [U], @noescape transform: U -> Result<V, Error>) -> Result<[V], Error>{
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d prefer a name somewhat less obscure than mapM (tho I acknowledge its Traversable heritage).

Perhaps all or allOrNothing?

Copy link
Contributor

Choose a reason for hiding this comment

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

A little further on this, and similar to the discussion of mapFilter above, we can probably decouple the essential action here, that of producing a success with all values if they are all successful, or failing otherwise, from the incidental action of mapping using transform, to get a slightly more idiomatic take on it:

public extension CollectionType where Generator.Element: ResultType {
    public func all() -> Result<[Generator.Element.Value], Generator.Element.Error> {
        …
    }
}

mapM’s full behaviour can be recovered by chaining after CollectionType.map:

let results = values.map(validate).all()

And you can use lazy, as well.

Copy link
Author

Choose a reason for hiding this comment

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

I agree that:

  1. Providing a lower level "all()" is better than a higher level of "mapM" here
  2. Due to the nature of "all()", lazy is preferred.

var vs = [V]()
for result in results{
let mapped = transform(result)
switch mapped{
case let .Success(value):
vs.append(value)
case let .Failure(error):
return Result(error: error)
}
}
return Result(vs)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think both of these would be more idiomatic if expressed as extension methods on CollectionType. Not only more idiomatic, but they’d also be available on collections other than Array.

23 changes: 23 additions & 0 deletions Tests/ResultTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,28 @@ final class ResultTests: XCTestCase {
let resultFailureRight = success &&& failure2
XCTAssert(resultFailureRight.error == error2)
}

func testMapFilterOnArray(){
let a = [1,2,3,4,5,6,7,8,9]
let r = mapFilter(a, transform: evenClosure)

XCTAssert(r == ["2","4","6","8"])
}

func testMapMOnArraySuccess(){
let a = [2,4,6,8]
if case let .Success(r) = mapM(a, transform: evenClosure){
XCTAssert(r == ["2","4","6","8"])
}else{
XCTFail()
}
}

func testMapMOnArrayFailure(){
let a = [2,4,5,8]
let r = mapM(a, transform: evenClosure)
XCTAssert(r.error == error)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for tests! 🙇

}


Expand All @@ -142,6 +164,7 @@ let error = NSError(domain: "com.antitypical.Result", code: 1, userInfo: nil)
let error2 = NSError(domain: "com.antitypical.Result", code: 2, userInfo: nil)
let failure = Result<String, NSError>.Failure(error)
let failure2 = Result<String, NSError>.Failure(error2)
let evenClosure = {i -> Result<String, NSError> in return i%2 == 0 ? .Success(String(i)) : .Failure(error)}


// MARK: - Helpers
Expand Down