Lazily initialise the properties of URLParser #146
Conversation
|
@nethraravindran I think all of the properties of |
nethraravindran
force-pushed
the
urlParser
branch
from
ed0c6b2 to
11f61d1
Compare
Codecov Report
@@ Coverage Diff @@
## master #146 +/- ##
=========================================
- Coverage 67.11% 66.71% -0.4%
=========================================
Files 20 20
Lines 1271 1298 +27
=========================================
+ Hits 853 866 +13
- Misses 418 432 +14
Continue to review full report at Codecov.
|
|
@pushkarnk Thank you! I have addressed the review comments. |
There was a problem hiding this comment.
Did you benchmark just delegating to each property to urlComponents? In other words, this:
- private lazy var _query: String? = urlComponents?.query
public var query: String? {
get {
return self.urlComponents?.query
}
set (newValue) {
- _query = newValue
+ self.urlComponents?.query = newValue
}
}Also, lazy in Swift is not thread-safe (see the "Note" at https://docs.swift.org/swift-book/LanguageGuide/Properties.html). Have you confirmed that instances of URLParser are never shared between threads?
Sources/KituraNet/URLParser.swift
Outdated
| self.queryParameters[$0.name] = $0.value | ||
| } | ||
| } | ||
| self.urlComponents = URLComponents(string: String(data: url, encoding: .utf8)!) |
There was a problem hiding this comment.
We shouldn't force-unwrap. If we cannot create the String then just set URLComponents to nil?
Sources/KituraNet/URLParser.swift
Outdated
| public var userinfo: String? | ||
| public var userinfo: String? { | ||
| get { | ||
| print("inside get") |
nethraravindran
force-pushed
the
urlParser
branch
4 times, most recently
from
2559683 to
2d644d4
Compare
|
@ianpartridge Thanks for the review! I've addressed the changes. But |
Sources/KituraNet/URLParser.swift
Outdated
|
|
||
| /// The userid and password if specified in the URL. | ||
| public var userinfo: String? | ||
| public var userinfo: String? { | ||
| var _userInfo: String? = nil |
There was a problem hiding this comment.
You can save a line here:
if let username = urlComponents?.user, let password = urlComponents?.password {
return "\(username):\(password)"
}
return nil
Sources/KituraNet/URLParser.swift
Outdated
|
|
||
| /// The query parameters broken out. | ||
| public var queryParameters: [String: String] = [:] | ||
| public var queryParameters: [String: String] { | ||
| var _query: [String: String] = [:] |
There was a problem hiding this comment.
This will calculate the queryParameters dictionary every time the property is accessed. For this property we should cache the dictionary.
There was a problem hiding this comment.
@ianpartridge I agree! Thanks for pointing this out :)
nethraravindran
force-pushed
the
urlParser
branch
from
2d644d4 to
17d720e
Compare
This increases the performance by 5-7% (approx)