Skip to content

Emit NSNotifications whenever objects are pinned and saved #319

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

Closed
fatuhoku opened this issue Sep 24, 2015 · 7 comments
Closed

Emit NSNotifications whenever objects are pinned and saved #319

fatuhoku opened this issue Sep 24, 2015 · 7 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@fatuhoku
Copy link

It's incredibly useful for the data layer of an application to fire off notifications whenever data is updated, such that UI components displaying that data knows to update itself. For instance, Core Data emits NSManagedObjectContextObjectsDidChangeNotification.

At present, the only way this can enforced for all saves and all pins is to swizzle all of the Parse SDK pin and save methods.

@nlutsenko
Copy link
Contributor

Great idea...
You are absolutely right that you can implement this right now, by simply overwriting pin/unpin/save/delete/fetch methods and continuing on them.

The thing with NSNotifications is that if you have a lot of them - they pollute a global notification space, so I am not sure about direct usage of notifications here. The multicast delegate system in my opinion would be a better fit.

cc @grantland @wangmengyan95 @richardjrossiii @hallucinogen
What do you guys think? It's a useful feature even beyond just iOS, something we were thinking for a long time with upcoming super-secret feature, but we can already build this one out.

@nlutsenko nlutsenko self-assigned this Sep 24, 2015
@hallucinogen
Copy link

It's useful and we already have such thing in .NET. Developers can listen to PropertyChanged event https://github.com/ParsePlatform/Parse-SDK-dotNET/blob/master/Parse/ParseObject.cs#L1772

@nlutsenko
Copy link
Contributor

Which gets us to the state where we probably should support KVO in the best manner possible.
@fatuhoku, what do you think?

@fatuhoku
Copy link
Author

@nlutsenko sure, though I would prefer NSNotification. The reason is that you aren't just watching one object. You're watching the whole class of objects for CRUD operations, which means those PFObjects you observe changes for may or may not exist on the LDS at all.

e.g. when I do a findObjectsInBackground new objects arrive from the backend, I can't exactly KVO for that.

The delegate pattern could work, as long as you have some specific namespace to register your delegates against — perhaps on a per-subclass basis. But delegates are rarely attached to classes... PFObjectSubclass.delegate = delegate looks weird and isn't very Cocoa. Parse doesn't yet have a concept of a DataContext object like in Core Data, to which you might want to tie this functionality.

So I think NSNotification is still a good fit, but I hear you — there's tonnes of them. Core Data uses them. Realm uses them. Given a long enough name with the right prefixes I can live with namespace pollution :)

@fatuhoku
Copy link
Author

@nlutsenko Oh, and I really honestly do hope that the super-secret feature you're talking about is bi-directional data replication for local datastore, so that we don't have to care about syncing data any more :)

@richardjrossiii
Copy link
Contributor

My biggest issue with NSNotificationCenter is how expensive it is, in terms of time.

Behind the scenes, NSNotificationCenter uses an NSDictionary with NSStrings as its backing store. The problem with that, is, that as you get lots and lots of notifications registered, you get an enormous amount of hash collisions, as NSString's -hash method simply returns the string's length.

Also, NSNotificationCenter callbacks (even on a specified queue) are blocking, which makes this entire situation even more complicated.

Combine that with the fact that LDS is already super slow (see #32), I'm not sure that at this time we should have any additions to LDS that could potentially reduce the performance even further.

That's just my opinion, but I will resort to whatever the rest of the team feels on this issue.

@stale
Copy link

stale bot commented Sep 19, 2018

This issue has been automatically marked as stale because it has not had recent activity. If you believe it should stay open, please let us know! As always, we encourage contributions, check out the Contributing Guide

@stale stale bot added the wontfix label Sep 19, 2018
@stale stale bot closed this as completed Sep 26, 2018
@mtrezza mtrezza added type:improvement type:feature New feature or improvement of existing feature and removed Discussion labels Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

5 participants