Type scheme once only #5
Conversation
|
Cool! I'm waiting WWDC keynote now. So I'll check this PR tomorrow 😅 |
|
Me too! Enjoy. |
ghost
changed the title
| @@ -3,21 +3,13 @@ import Foundation | |||
| public typealias SimpleRouter = Router<Void> | |||
|
|
|||
| public final class Router<UserInfo> { | |||
| public let scheme: String | |||
| private let scheme: String | |||
There was a problem hiding this comment.
Maybe, I guess that nobody use Router.scheme from external. 🤔
There was a problem hiding this comment.
I think so. I think it might be good to release API, once it becomes useful.
| @@ -22,6 +22,23 @@ final class RouterTest: XCTestCase { | |||
| XCTAssertFalse(router.responds(to: URL(string: "notfoobar://aaa/bbb")!)) | |||
| XCTAssertTrue(router.responds(to: URL(string: "foobar://spam/ham")!)) | |||
| } | |||
|
|
|||
| func testCanRespond2() { | |||
There was a problem hiding this comment.
testCanRespondWithoutScheme and others
Sources/Crossroad/Router.swift
Outdated
|
|
||
| internal func register(_ route: Route<UserInfo>) { | ||
| if scheme != route.patternURL.scheme { | ||
| assertionFailure("Router and pattern must have the same schemes. expect: \(scheme), actual: \(route.patternURL.scheme)") |
| @@ -22,6 +22,23 @@ final class RouterTest: XCTestCase { | |||
| XCTAssertFalse(router.responds(to: URL(string: "notfoobar://aaa/bbb")!)) | |||
| XCTAssertTrue(router.responds(to: URL(string: "foobar://spam/ham")!)) | |||
| } | |||
|
|
|||
| func testCanRespond2() { | |||
| let router = SimpleRouter(scheme: schema) | |||
There was a problem hiding this comment.
Oh, I found typo. Could you fix them?
/schema/scheme/
Sources/Crossroad/Router.swift
Outdated
| @@ -35,12 +27,17 @@ public final class Router<UserInfo> { | |||
|
|
|||
| public func register(_ routes: [(String, Route<UserInfo>.Handler)]) { | |||
| for (pattern, handler) in routes { | |||
| var pattern = pattern | |||
There was a problem hiding this comment.
You can write more clearly code.
public func register(_ routes: [(String, Route<UserInfo>.Handler)]) {
for (pattern, handler) in routes {
let patternURLString: String
if pattern.hasPrefix("\(scheme)://") {
patternURLString = pattern
} else {
patternURLString = "\(scheme)://\(pattern)"
}
guard let patternURL = PatternURL(string: patternURLString) else {
assertionFailure("\(pattern) is invalid")
continue
}
let route = Route(pattern: patternURL, handler: handler)
register(route)
}
}There was a problem hiding this comment.
I think you needn't remove internal func register(_ route: Route<UserInfo>).
| XCTAssertTrue(router.responds(to: URL(string: "foobar://foo")!)) | ||
| XCTAssertTrue(router.responds(to: URL(string: "foobar://foo/bar")!)) | ||
| XCTAssertTrue(router.responds(to: URL(string: "foobar://foo/10000")!)) | ||
| XCTAssertFalse(router.responds(to: URL(string: "notfoobar://aaa/bbb")!)) |
There was a problem hiding this comment.
Could you add assertions to check without scheme patterns?
XCTAssertFalse(router.responds(to: URL(string: "static")!))
XCTAssertFalse(router.responds(to: URL(string: "foo/bar")!))
XCTAssertFalse(router.responds(to: URL(string: "spam/ham")!))
|
Have made the changes @giginet suggested 👍 |
There was a problem hiding this comment.
Great job! 👍
I'll merge this after correcting lint warnings.
https://travis-ci.org/giginet/Crossroad/jobs/388382200#L604
Thank you! 🎉
Sources/Crossroad/Router.swift
Outdated
| private var routes: [Route<UserInfo>] = [] | ||
|
|
||
| public init(scheme: String) { | ||
| self.scheme = scheme | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Could you execute swiftlint autocorrect?
|
You can ignore |
I would typically have a component assembler in my codebase somewhere that would interact with Crossroads.
The assembler would build paths.
Consuming the scheme once makes it a bit easier to manage an assembler and I want to avoid typing strings often.
See this pull request in action here