Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rename HTTPHeaders.getCanonicalForm to subject and improve efficiency #293

Merged
merged 1 commit into from
Apr 9, 2018

Conversation

weissi
Copy link
Member

@weissi weissi commented Apr 9, 2018

Motivation:

HTTPHeaders had an unusual API for Swift: getCanonicalForm(String) which is
better suited as a subscript [canonicalForm: String].
Also the implementation was needlessly inefficient.

Modifications:

  • renamed HTTPHeaders.getCanonicalForm to HTTPHeaders[canonicalForm]
  • improved efficiency by replacing anti-pattern
    Array.map { ... }.reduce(+, []) by flatMap

Result:

more Swift in both meanings

Motivation:

HTTPHeaders had an unusual API for Swift: `getCanonicalForm(String)` which is
better suited as a subscript `[canonicalForm: String]`.
Also the implementation was needlessly inefficient.

Modifications:

- renamed HTTPHeaders.getCanonicalForm to HTTPHeaders[canonicalForm]
- improved efficiency by replacing anti-pattern
  Array.map { ...  }.reduce(+, []) by flatMap

Result:

more Swift in both meanings
@weissi weissi requested review from normanmaurer and Lukasa April 9, 2018 06:50
// It's not safe to split Set-Cookie on comma.
let queryName = name.lowercased()
if queryName == "set-cookie" {
return self[name]
}

if let result = storage[queryName] {
return result.map { tuple in tuple.1.split(separator: ",").map { String($0.trimWhitespace()) } }.reduce([], +)
return result.flatMap { tuple in tuple.1.split(separator: ",").map { String($0.trimWhitespace()) } }
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the only non-renaming change

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Apr 9, 2018
@Lukasa Lukasa added this to the 1.4.0 milestone Apr 9, 2018
@Lukasa Lukasa merged commit 7e70bf5 into apple:master Apr 9, 2018
@weissi weissi deleted the jw-canonical-form-efficiency branch April 9, 2018 07:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants