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

Threading is bad style/inefficient #91

Open
uliwitness opened this issue Mar 3, 2017 · 2 comments
Open

Threading is bad style/inefficient #91

uliwitness opened this issue Mar 3, 2017 · 2 comments

Comments

@uliwitness
Copy link

This code could use some improvement: Currently it starts its own thread to download the data, then starts a synchronous request (which in turn starts its own thread and blocks until that returns). What this should really do is just use one of the asynchronous networking calls (or even better, use NSURLSession's asynchronous calls to fetch the current app version from the app store)

It also does several performSelectorOnMainThread: calls after one another. Each such call has an overhead. It would be much cleaner if it just did one performSelectorOnMainThread: and had that call one method that does the 4 or so calls that need to be on the main thread.

Finally, the way it creates the singleton in +load with performSelectorOnMainThread: waitUntilDone: NO has a race condition. This should really just be using dispatch_once like one does these days to set up a singleton thread-safely.

@uliwitness
Copy link
Author

PS - I found this project via your answer to http://stackoverflow.com/questions/6256748/check-if-my-app-has-a-new-version-on-appstore and it was the only answer that wasn't flat-out wrong like most of the others. I'm not actually using this code, or I would have sent you a pull request with the fixes instead of just filing bugs asking you to do it.

@nicklockwood
Copy link
Owner

@uliwitness I don't disagree. This code predates GCD (iOS 3.x), and has no unit tests, so there's a significant cost/benefit issue with refactoring it. If I ever need to use it myself in a new application, I'll consider a rewrite.

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

No branches or pull requests

2 participants