As a part of going through the design patterns we've found in the creation of the Artsy iOS apps, I'd like to talk a bit about some of the patterns that we've had and migrated away from. This is not 100% comprehensive, as there has been a lot of time, and a lot of people involved. Instead I'm going to try and give a birds eye view, and zoom in on some things that feel more important overall.
It's important to preface that I don't believe in perfect code, or am I a fan of big re-writes. We can spot a bad pattern, but not do anything about it. We do have apps to ship, not a codebase to make perfect for the sake of technical purity.
NSNotifications as a decoupling method
A lot of the initial codebase for Energy relied on using NSNotifications as a way of passing messages throughout the application. There were notifications for user settings changes, download status updates, anything related to authentication and the corresponding different error states and a few app features. These relied on sending global notifications with very little attempts at scoping the relationship between objects.
NSNotificationCenter notifications are an implementation of the Observer Pattern in Cocoa. They are a beginner to intermediate programmer's design paradigm dream. It offers a way to have objects send messages to each other without having to go through any real coupling. As someone just starting with writing apps for iOS, it was an easy choice to adapt.
One of the biggest downsides of using NSNotifications are that they make it easy to be lazy as a programmer. It allows you to not think carefully about the relationships between your objects and instead to pretend that they are loosely coupled, when instead they are coupled but via stringly typed notifications.
Loose-coupling can have it's place but without being careful there is no scope on what could be listening to any notification. Also de-registering for interest can be a tricky thing to learn and the default memory-management behavior is about to change ( for the better.)
We still have a lot of notifications in Energy, however in Eigen and Eidolon there are next to none. We don't even have a specific file for the constants.
Not much to say here, using
#defines as constants was definitely in favour when I learned Objective-C. Likely a throw back from C. Using
#defines as constants would not use on-device memory to store an unchanging value. This is because a
#define uses the pre-compiler to directly change the source code to be the value, whereas using a static constant takes up memory space on the device, which we used to really care about. It's likely that a modern copy of LLVM doesn't assign on device memory unless it needs to, especially for things marked
const. Switching to real variables means you can inspect and use in a debugger and use can rely on the type system better.
Where we used to have this:
1 2 3 4 5 6 7 8 9 10 11 12 13 14
We would instead build something like this:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
This gives us the ability to not sprinkle analytics code around the app in every file. It keeps the responsibilities of objects simpler and we've been happy with it in Eigen. We've not migrated it into Energy, its dependency on ReactiveCocoa brings too much additional weight. So far we've been applying analytics inline, Energy has much less need for individual analytics throughout the application. If you want to learn more about this pattern check out Aspect-Oriented Programming and ARAnalytics.
Class Methods as the whole API
For a very long time, I preferred aesthetics of class based APIs. E.g. using only class methods instead instance methods. I think I still do. However, once you start adding tests to a project then they become a bit of a problem.
I'm a big fan of the idea of Dependency Injection within tests. This, roughly TL:DR'd, means passing in any additional context, instead of an object finding the context itself. A common case is a call to
NSUserDefaults. It's very likely not the role of your class to know which
NSUserDefaults object you're working with, but it's likely that you're making that decision in the method by doing something like
[[NSUserDefaults standardUserDefaults] setObject:reminderID forKey:@"ARReminderID"];. Using dependency injection would be allowing that object to come from outside that method. If you're interested in a longer and better, explanation, read this great objc.io by Jon Reid.
The problem with a class based API, is that it becomes difficult to inject that object. This doesn't flow well with writing simple, fast tests. You can use a mocking library to fake the class API, but that feels weird. Mocking should be used for things you don't control. You control the object if you're making the API. Having an instance'd object means being able to provide different versions with different behaviors or values, even better if you can reduce the behavior to a protocol.
Objects Sneakily Networking
When you have a complicated application, there can be a lot of places you can perform networking. We've had it in models, view controllers and views. Basically throwing the idea of purity in MVC out of the system. We started to recognise a pattern in eigen, we were not doing a good job of keeping our networking well abstracted. If you want to see the full story check out the moya/Moya README.
One attempt to at trying to fix this pattern by creating a different type of networking client I've just referenced, Moya.
The other was to abstract any networking performed into a separate object. If you've heard of Model-View-ViewModel (MVVM) then this is a similar premise for networking instead of views. Network Models give us a way to abstract the networking into a set of behaviors. The extra abstraction means that you think "I want the things related to x" instead of "send a GET to address x and turn it into y."
Network models also make it extremely easy to swap behavior out in tests. In eigen, we have our asynchronous networking run synchronously in tests but we still use the network models to be able to provide whatever data we want to expect from the server in our tests.
Subclassing more than twice
As projects evolve it can become very easy to subclass x in order to provide a "similar but a little bit different" behavior. Perhaps you need to override some methods, or add a specific behavior. Like the urban myth of a frog being slowly boiled, you end up with a difficult to understand codebase as expected behavior mutates depending on how deep the hierarchy goes.
One pattern for handling this is class composition. This is the idea that instead of having one object do multiple things, you allow a collection of objects to work together. Providing more space for each object to conform to the Single Responsibility Principle (SRP.) If you're interested in this, you may also be interested in the class clusters pattern.
A good example of this comes from Energy, our root view controller
ARTopViewController used to control its own toolbar items. Over 4 years this became difficult to manage, and a lot of extra code in the view controller. By abstracting out the implementation details of managing the toolbar items into it's own class we were able to allow the
ARTopViewController to state what it wanted by not how it was done.
Configuration Classes over Inter-Class Communication
A one of the most important aspects of Energy is to email artworks. So there is a lot of code around configuring the email you want to send, and then generating HTML from those settings. This started out pretty simple as we had very few app-wide settings. Over time, deciding what we need to show in terms of settings and how they affected the email became very complicated.
The part that eventually became a strong enough code-smell to warrant a re-write was the view controller which allowed a partner to choose what information to send would pass details individually to an object that would generate the HTML. I found it difficult to write simple tests for the class' behavior. Initially I would mock the email composer, then inspect the methods that were called. This felt wrong, as you shouldn't really be mocking classes you own. Given the importance of the functionality that classes provide our application, ideas on ways to improve the section of code stayed on my mind for a long time.
The fix came to me during Justin Searls' talk Sometimes a Controller is Just a Controller - specifically slide 55. Justin talks about objects that either hold and describe a value or perform useful behavior, never both.
I took this advice and re-evaluated the relationship between settings view controller and composer object. Before the change, the settings view controller would configure the composer directly. Now, the settings view controller creates a configuration object and the composer consumes it. This made it significantly easier to write tests for both objects, as they have very obvious inputs and outputs in the form of a AREmailSettings. The AREmailComposerTests in particular become much more elegant.
Direct use of Responder Chain
Before I worked at Artsy, I was a Mac developer, I've been doing that since before iOS existed, so this influences my code style. One of the great parts of the Cocoa toolchain is the responder chain, a well documented way of passing methods up a known chain of objects. It solves a common problem with complicated view structures. You could have a button that is generated at runtime deep inside a view hierarchy and you would like the view controller to handle it being tapped. You could use a long chain of delegate methods, or use a private method to get the reference to the view controller instance. On the Mac usage of the responder chain is a common pattern, on iOS it is used rarely.
We have this problem with our Artwork view controller in Eigen. There are buttons that are many stack views deep that need to pass a message back to the view controller. When we first hit this the issue I immediately used the responder chain, you write a line of code like:
[bidButton addTarget:self action:@selector(tappedBidButton:) forControlEvents:UIControlEventTouchUpInside]; where the
self is referring to the view. This would send the message
tappedBidButton: up the responder chain where it is reacted upon by the ARArtworkViewController.
I had to explain the premise of the responder chain to almost everyone touching this area of the code base. This is great in terms of the "lucky 10,000" but means that the pattern is unintuitive to those who have not previously heard of it. There was one more issue, the lack of coupling means that renaming selectors via refactoring can break the chain.
The way that we reduced the cognitive load was via a protocol, all of the actions that the responder chain will use are mapped inside ARArtworkActionsViewButtonDelegate-like protocols. It's a bit of a white-lie given that there is no direct relationship using the protocol in the app, but it makes the relationship more obvious. We use a class extension that conforms to these types of protocols to keep the actions all kept in one place.
There are many design patterns, and they all come with trade-offs. Over time, our opinions on what is "good code" changes, this is great. It's important that as developers we understand that being able to change our minds is one of the most vital skills we have in our toolchain. This means being open to opinions outside of your usual sphere of influence and to maybe bring some good ideas from them. It's great to be passionate about an aspect of how we craft applications, but from my perspective, the best programmers choose pragmatism over idealism.