0
\$\begingroup\$

In our swift project, there are multiple service classes that encapsulate functions, mostly network-api calls. They implement a protocol, and depend on other service classes, e.g. an authorization service or a network facade.

Their instances should only exist once, hence they are designed as singletons, with a private constructor and a single point to access, like so:

static let defaultInstance = MyManagerImplementation(networkFacade: MyApiFacade())

In order to write unit tests for each service class, i want to mock the dependencies. Sadly this is not possible in the code above.

So i re-wrote the initialization like this.

let networkFacade: MyApiFacade

private static var instance: MyManagerImplementation?

static func defaultInstance(apiFacade apiFacade: MyApiFacade = RestApiImplementation.defaultInstance) -> MyManager {
    if instance == nil {
        instance = MyManagerImplementation(networkFacade: apiFacade)
    }
    return instance!
}

init(networkFacade: MyApiFacade) {
    self.networkFacade = networkFacade
}

Which should be the classic singleton pattern, as i know it from other languages.

The problem i have with this is that swift seemingly wants to avoid this way of defining a singleton. It is way longer than the single-line-singleton, which is considered best practice in many posts i found on the web.

So, would it make sense to you to define these service-classes completely static? Or would you go with another solution or the solution above?

\$\endgroup\$
1
\$\begingroup\$

Your singleton pattern leaves a lot to be desired.

  • It is not thread safe.
  • It makes use of the crash operator (!)
  • The class can be initialized any number of times--this isn't a true singleton
  • The fact that defaultInstance is a function which can be passed a parameter can lead to misleading results.

Thread-safety

The simplest way to make singletons thread safe in Swift is pretty straight-foward.

class MySingleton {
    private init(){}
    static let sharedInstance = MySingleton()
}

But given we want our singleton to potentially have different types, we need a slightly more complicated pattern.

Without account for any of the other points I want to address, the simplest solution looks like this:

static func defaultInstance(apiFacade apiFacade: MyApiFacade = RestApiImplementation.defaultInstance) -> MyManager {

    struct Static {
        static var onceToken: dispatch_once_t = 0
        static var instance: MyManagerImplementation?
    }

    dispatch_once(&Static.onceToken) {
        Static.instance = MyManagerImplementation(networkFacade: apiFacade)
    }

    return Static.instance!
}

The Crash Operator

Force unwrapping optional variables always leaves us with the potential to crash. Even in a case like this where crashing probably is the correct behavior (because for the variable to be nil is presumably impossible), we still deserve better diagnostic information in the case of the crash.

By simply changing the last line from return Static.instance! to this:

guard let instance = Static.instance else {
    fatalError("Unable to obtain reference to MyManager singleton instance.")
}

return instance

We're doing the exact same thing, but in the case of a crash, we get better diagnostic information.

Importantly, if for some reason in the future, the MyManagerImplementation(networkFacade:) constructor were ever turned into a failable initializer, your app is all of the sudden going to start crashing without Xcode providing any warnings here.

And if left unchanged, you'll have no diagnostic information beyond:

fatal error: Unexpectedly found nil while unwrapping an optional

So... a few extra lines can be very helpful.


Not a true singleton

Because your initializer is perfectly accessible outside of the file, this is not a true singleton. You do have a shared instance available, but other instances can be instantiated.

And perhaps importantly, if you compare your original implementation to the one posted above with the nested struct, even though your instance property is private, it has to be declared as a var (let wouldn't work), and that means it is possible to change the reference.

This is important to note because you can't stop users from capturing a reference to the original instance and continuing to make calls on it even after you've changed the value the singleton returns.


Misleading Results

Your defaultInstance function accepts and argument, and that argument has a default value of RestApiImplementation.defaultInstance. This works for allowing your first time instantiation to choose how the singleton is set up.

But... we're going to get misleading results if the first time we called it we passed one sort of implementation, and then later we tried calling it with the other type of implementation. Your code doesn't change the implementation, and despite me calling it requesting it be set up with Foo, you return me a Bar.

Perhaps better would be a method to initialize the singleton with a particular value? And then you have to decide what happens if initialize is called more than once. On the second and later calls, you must either reinitialize with the new value or you should do something to indicate that the action the user requested wasn't able to be performed. So perhaps, you should throw an error here.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.