My friend Soroush posted this week an article about the architecture he developed for push notification handling in his app. I’ve written about singletons before here (twice), and while I like his architecture overall I would recommend some small changes to facilitate maintainability and testability.
Of course, you should read his post for context before reading this one.
Allow creation and usage of standalone instances
I agree that using a singleton instance of
RGPushCenter in application code is the right choice. It would never make sense to have more than one push center in an app, and it feels intuitively aligned with
UIApplication — it’s part of the application’s operating context.
However, it’s important not to discourage the standard
-init pattern for instance creation. While a singleton instance should be used in application code, unit tests will need to create and use multiple push centers.
I would go so far as to explicitly separate the singleton
+sharedCenter method, perhaps in its own category. This concern (singleton-ism) is secondary and unrelated to the real responsibility of the
Incidentally, I thought
+sharedCenter was an excellent name for the method.
Ensure the class obeys the single responsibility principle
Write a simple, clear statement of responsibility for the push center class. Document it in the class’s header.
Obeying the SRP is always a good idea, but it’s absolutely crucial when implementing a singleton. Because singletons are globally accessible, it’s tempting and easy to keep adding useful bits of functionality instead of carefully considering where those responsibilities really lie.
Having a clear statement of responsibility and enforcing it during code reviews is critical to keep singleton classes from becoming unmanageable.
Consider extracting registration concerns
I think the concerns around communicating with the APNs and Genius API (registering for notifications and transforming notification dictionaries into instances of a model class) could be extracted into a separate class.
RGPushCenter is concerned with responding to notifications; bridging between the Apple and
RG worlds is a separate responsibility.
I haven’t thought through this design yet, though, so I’ll refrain from including this change in my interface outline below.
Eliminate awareness of other singletons
It sounds to me like the push center has a dependency on the RGNavigationManager singleton. Find a way to eliminate this coupling.
I would start by extracting the method(s) the push center needs into a protocol, say
@protocol RGPushNavigator. Then make
RGNavigationManager comply to that protocol, perhaps in an app-specific category so the navigation manager doesn’t explicitly know about the push center. Finally, have a “navigation delegate” on the push center, and in the application delegate set that property to the shared navigation manager.
That may sound like unnecessary complexity. But dependencies like this one between singleton classes can very quickly lead to an unmanageable web of dependencies. This makes it impossible to extract code for reuse in another project, and it inhibits proper testing.
Using dependency injection — here, using a delegate pattern and requiring a protocol instead of a concrete class — makes testing standalone instances of
RGPushCenter easy and will allow easy reuse in future projects.
The resulting interfaces
Based on my comments, here’s how I would rewrite the interfaces for
RGPushCenter and its associated types. I’ve excluded interfaces I wouldn’t change (eg.
(I’ve written before that interfaces are the most important part of this design process.)
@interface RGPushCenter @property (nonatomic, weak) id<RGPushCenterNavigationDelegate> navigationDelegate; #pragma mark - Responding to Notifications - (void)presentToastWithUserInfo:(NSDictionary *)userInfo; - (void)performRoutingForNotificationWithUserInfo:(NSDictionary *)userInfo; - (void)addDelegate:(id<RGPushCenterDelegate>)delegate; - (void)removeDelegate:(id<RGPushCenterDelegate>)delegate; #pragma mark - Registering for Notifications // As noted above in this blog post, this coordination should probably be extracted to a different method. - (void)registerForRemoteNotifications; @end @protocol RGPushCenterNavigationDelegate - (void)openURL:(NSURL *)url; @end
@interface RGPushCenter (Singleton) + (instancetype)sharedCenter; @end
RGNavigationManager+PushSupport.h (app-level, not part of the push center library)
@interface RGNavigationManager (PushSupport) <RGPushCenterNavigationDelegate> @end
RGPushCenter, alongside any of its collaborating types, is a prime candidate for unit testing, and the changes I recommended above will facilitate testing. It’s now trivial to create and configure instances for testing, and those instances only have injected dependencies which could easily be ignored or mocked.
I thought Soroush’s architecture was fine; singletons sometimes do make sense in our applications, and this is one of these times.
Unfortunately, many common patterns lead too quickly to unmaintainable, untestable code when used when singletons. These small recommendations will help keep this app’s codebase clean, and they’ll help keep future programmers on the project happy.
As always, I welcome discussion and feedback; I’m @cdzombak on Twitter.