Skip to content

Improve clarity of asyncAfter code and other timeout#259

Merged
daltoniam merged 1 commit into
daltoniam:masterfrom
robinkunde:master
Sep 30, 2016
Merged

Improve clarity of asyncAfter code and other timeout#259
daltoniam merged 1 commit into
daltoniam:masterfrom
robinkunde:master

Conversation

@robinkunde

Copy link
Copy Markdown
Contributor

Hi, I'd like to improve the clarity of the deadline calculation for Dispatch.asyncAfter in WebSocket.disconnect(). I think the existing code is a result of the Swift3 code updater and could use some syntactic sugar.

I also added underscores to time-related constants to improve legibility.

Comment thread Source/WebSocket.swift
case .some(let seconds) where seconds > 0:
callbackQueue.asyncAfter(deadline: DispatchTime.now() + Double(Int64(seconds * Double(NSEC_PER_SEC))) / Double(NSEC_PER_SEC)) { [weak self] in
let milliseconds = Int(seconds * 1_000)
callbackQueue.asyncAfter(deadline: .now() + .milliseconds(milliseconds)) { [weak self] in

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You already have seconds to use now() + .seconds(Int(x)) function.

@robinkunde robinkunde Sep 28, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That would remove the ability to specify fractional timeouts like 2.5 seconds. Well, you could pass them in, but the fractional part would be silently dropped. That's why I'm doing the conversion to milliseconds first.

@daltoniam

Copy link
Copy Markdown
Owner

It was and I like the clarity changes. Thanks!

@daltoniam daltoniam merged commit b0fa08c into daltoniam:master Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants