The following question arose in my mentorship Slack (a monthly subscription program that you can subscribe and unsubscribe on a monthly basis).
Question: Hey Ray, hope you’re doing fine. I got another question. I’m on this one project with MVC. We have 2 very heavy services that are the heart of the app. The DB-Service which communicates with Realm and handles things like updating objects, checking if an update is needed, delete objects, etc. On the other hand, there is the NetworkService which checks if a connection is there before making a request. If not, it returns local content if available and does the usual downloading of content and handing the stuff over to the DB-Service.
Both of these files are singletons and they are 700 and 900 lines of code. Most of the time very complicated code with a lot of conditionals, e.g. in plain language:
Get this collection and all the stuff that is needed for it. If it has a parent download the parent of it and the main content of it. Download all the resources but only once. If there is no connection try to get offline content. If there is a connection, check if the current stored content is up to date and then don’t download anything, etc., etc.
Now, we want to refactor those services. But before I would like to add unit tests as then we can verify if it has worked correctly before and also afterward. But I have seen that this is not really possible in the way that the code works right now. So I need to make the refactoring upfront. Is there a good way to split large services into smaller chunks and remove singletons?
I thought about dependency injection but don’t know if this is a good idea. Overall, there are 8 VCs that use the DB-Service. It’s maybe also possible to split the services to smaller chunks by making one service per VC. That’s maybe more reasonable and can be combined with the dependency injection.
Answer: Adding unit tests to bad code doesn’t help, as you’ve already known. There is simply no way to write good unit tests easily for bad code. By doing so, you’ll just become more frustrated with the existing code, and the resulting tests will be bad code too. So you’ll end up with 2x bad code.
As you can see from the Wistia videos. Instead of writing unit tests for the existing app, I just redo (not refactor) one feature, one scene at a time while adding unit tests for this new code. The trick is to not modify the existing scene, but to create a new scene with a different name and port one feature at a time, keeping it small. This allows you to quickly switch between the old and new scene, while giving you the total freedom to do whatever with the new scene.
Note that this doesn’t mean it’s a complete rewrite either. When you’re porting one feature at a time, you’ll probably find that you can simply copy and paste a lot of code from the old class to the new class. Make sure any public API methods are thoroughly unit tested while you’re doing this. Since you already figure out the high-level use cases, you can base your new classes on these use cases and the edge conditions.
You mentioned you’re dealing with large service classes, not scenes. But the same concept for scenes still applies to services and workers. I would create a new worker and start porting one tiny feature at a time. In the process, keep you eyes open for multiple workers. I’m sure you’ll find out a large service class will be better to be broken up into multiple ones. For example, at the very least, there should be a class to check for networking connection, a class to do the actual download, a class to check if the data is up to date, a class to persist to DB, …etc.
Regarding your concern about singletons, singletons by themselves aren’t the real problem. It’s the misuse of singletons that’s the problem. There are legitimate use cases for singletons.
Regarding dependency injection, the old code isn’t isolated in a way that facilitates dependency injection. You may be able to modify it to allow some dependency injection. But 99% of the code will not be kept, and thus the time will be wasted anyway. I recommend against that.
