Skip to content
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.

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

swift 3.0 #1

Merged
merged 4 commits into from
Dec 13, 2016
Merged

swift 3.0 #1

merged 4 commits into from
Dec 13, 2016

Conversation

asalom
Copy link
Owner

@asalom asalom commented Dec 13, 2016

No description provided.

public func async (callback: Void -> Void) {
dispatch_async(dispatch_queue) { callback() }
open func async (_ callback: @escaping (Void) -> Void) {
dispatch_queue.async { callback() }
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor thing, but can we do dispatch_queue.async(callback)?

Copy link
Owner Author

Choose a reason for hiding this comment

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

we can do dispatch_queue.async(execute: callback)

if isCurrent { callback(); return } // prevent deadlocks!
dispatch_sync(dispatch_queue) { callback() }
dispatch_queue.sync { callback() }
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

}

public func async <T> (callback: T -> Void) -> T -> Void {
open func async <T> (_ callback: @escaping (T) -> Void) -> (T) -> Void {
return { [weak self] value in
if self == nil { return }
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we convert this to guard let or is it too overkill? :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

actually can we do self?.async { callback(value) }?

Copy link
Owner Author

Choose a reason for hiding this comment

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

done, actually i removed too the force unwrap from bellow

return { [weak self] value in
if self == nil { return }
self!.async { callback(value) }
}
}

public func sync <T> (callback: T -> Void) -> T -> Void {
open func sync <T> (_ callback: @escaping (T) -> Void) -> (T) -> Void {
return { [weak self] value in
if self == nil { return }
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too

Copy link
Owner Author

Choose a reason for hiding this comment

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

done




// MARK: Internal

init (_ queue: dispatch_queue_t) {
init (_ queue: Dispatch.DispatchQueue) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need the Dispatch. prefix here and in the other usages?

Copy link
Owner Author

Choose a reason for hiding this comment

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

removed

remember()
}

func remember () {
dispatch_queue_set_specific(dispatch_queue, &kCurrentQueue, getMutablePointer(self), nil)
guard let mutablePoiner = getMutablePointer(self) else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

small typo here Poiner

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

@@ -2,15 +2,15 @@
import Foundation
import UIKit

public typealias Timer = DispatchTimer
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't this dangerous? Can it collide with NSTimer -> Timer?

Copy link
Owner Author

Choose a reason for hiding this comment

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

i can remove it all together but then we'll break the public API of the framework. i don't think it matters actually

Copy link
Owner Author

Choose a reason for hiding this comment

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

i removed this, Group and Dispatch typealiases

dispatch_source_set_timer(timer, time, UInt64(delay_ns), UInt64(tolerance * CGFloat(NSEC_PER_SEC)))
dispatch_source_set_event_handler(timer) { [weak self] in let _ = self?.fire() }
dispatch_resume(timer)
let time = DispatchTime.now() + Double(Int64(delay_ns)) / Double(NSEC_PER_SEC)
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we don't need to divide by Double(NSEC_PER_SEC) if in the line before we don't multiply by CGFloat(NSEC_PER_SEC)

let time = DispatchTime.now() + Double(Int64(delay_ns)) / Double(NSEC_PER_SEC)

let interval = DispatchTimeInterval.nanoseconds(Int(delay_ns))
let leeway = DispatchTimeInterval.nanoseconds(Int(UInt64(tolerance * CGFloat(NSEC_PER_SEC))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can avoid double converting to UInt64 and Int

Copy link
Owner Author

Choose a reason for hiding this comment

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

are we sure that tolerance * CGFloat(NSEC_PER_SEC) will always be positive?

Copy link
Collaborator

Choose a reason for hiding this comment

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

you're converting to UInt64 anyway

isConcurrent = true
dispatch_queue = dispatch_get_global_queue(priority, 0)
dispatch_queue = DispatchQueue.global(qos: qos)
remember()
}

init (_ concurrent: Bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we remove the _ so that the concurrent argument is explicit?

Copy link
Owner Author

Choose a reason for hiding this comment

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

i removed all of them on this class

podboq
podboq previously approved these changes Dec 13, 2016
@asalom asalom merged commit e1754a3 into master Dec 13, 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
3 participants