Skip to content
This repository has been archived by the owner on Jun 8, 2019. It is now read-only.

Added Swift version of the code for better readability and just for the fun of it. #11

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

eyaldar
Copy link

@eyaldar eyaldar commented Jan 2, 2016

Hey,
I stumbled upon your code while learning Swift and looking for clustering abilities,
In order to better understand your solution I migrated your code to a Swift solution.
As it is already implemented I thought others like me(people with no knowledge of the Objective-C language nor willing to learn it) might appreciate the ability to read the same code in the Swift language.

@@ -43,7 +43,7 @@ class ViewController: UIViewController, MKMapViewDelegate {
let toRemove = NSMutableSet(set: before)
toRemove.minusSet(after as Set<NSObject>)

dispatch_async(dispatch_get_main_queue()) { () -> Void in
NSOperationQueue().addOperationWithBlock() {

Choose a reason for hiding this comment

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

You are creating a new NSOperationQueue and modifying the mapView while running on that new queue. The mapView, like all UIViews, should only be modified on the main queue/thread.

Copy link
Author

Choose a reason for hiding this comment

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

Wouldn't the potential problem you're describing cause the app to crash if the code was actually changing the view from another thread? am I'm missing something?

Choose a reason for hiding this comment

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

I've seen several occasions where views where modified on a background thread and it did not lead to a crash. It usually results in weird behavior.

Copy link
Author

Choose a reason for hiding this comment

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

Oh wait, it seems that I actually fixed this bug on the next commit:
be4bdcd

Choose a reason for hiding this comment

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

I'm sorry, you are right.

However, NSOperationQueue.mainQueue().addOperationWithBlock() is essentially the same as dispatch_async(dispatch_get_main_queue()). I personally prefer dispatch queues and in Swift 3 libdispatch got a really nice "swifty" interface, see https://github.com/apple/swift-evolution/blob/master/proposals/0088-libdispatch-for-swift3.md

Copy link
Author

Choose a reason for hiding this comment

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

Good to know :), I'm still learning Swift - that was part of the reason I decided to translate the code - it helped me understand the clustering algorithm better and try out coding in Swift.
Also I find Objective-C's syntax repulsive :P

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants