4

TL;DR

Part 1)

I want to refactor the biggest part of the app on which I work - and it is pretty much spaghetti. It's a single class that makes the requests to the server, parses the JSON, and saves the data in the database (it also has knowledge of some UI that needs to change when specific business rules are met and it definitely shouldn't know all this stuff). It has somewhere below 5000 LOC :(.

One way of breaking the class in smaller pieces would be to separate the requests by semantics (for example User requests, Search requests etc). Would that be useful or not really?

Also, I think that networking code should be decoupled from the parsing code and also from the DB code. The problem is that I don't really know what approach (architecture/design pattern) should I use to do this the best way. (for example, our JSON parsing library is old and is not maintained anymore - we wanted to change it because it produced some problems and we were stuck because of the tight-coupling).

Part 2)

I also would like to receive some suggestions for the following issue: We use the DB mostly for offline mode & caching. The user can see data in offline mode, modify it (and requests get serialized) and when he gets back online, the requests will be made to the server. We also use it as a cache to show something on the UI while we do a request in the background to see if there is new/modified data (we use a similar mechanism like ETag to check with the server).

So this is the flow when a user enters a screen:

enter_screen -> 
check_cache + background_request -> 
show_UI with cache data -> 
update_UI when the background request finishes and new/modified data is available.

Can this stuff be abstracted away somehow? I was thinking of some kind of delegate pattern where I get an immediate callback when I hit the cache and then get a later callback when the request finishes (with optional flags to distinguish between cache and fresh data).


A bit of a background story

I currently work for an iOS app that is aprox. 4 years old. I only joined the team a year ago. In my opinion, the code is ugly, full of hacks, and has a very shaky architecture. We are 2 iOS developers working on it. The client doesn't have the technical expertise and may not understand all the implications of some issues that we have. Making a client understand why "refactoring" is needed, is a hard task. I struggle to refactor how much I can, but we still have a lot to go trough.

The app also has a lot of features that increases the complexity of the app: deeplinks, push notifications, offline mode, non blocking UI etc. all combined with a rather large and hairy data model.

Because the client requests features in a constant manner, I feel that we have a lot of technical debt.

The mobile world is changing pretty fast and new technologies appear (Size classes, Auto Layout, Swift, etc.). This is true for all technologies, but I somehow feel that the race is a bit more intense in the mobile world. Currently, I only dream of using Swift at work (although I play with it at home in my spare time).

1
  • 1
    There appears to be multiple questions here, as well as unnecessary information. I would focus on architectures using layering. Server requests, parsing, and saving to the database (serialization) could all be independent of one another, in lower layers. It seems you already figured out the best solutions, based on the statement of the questions. Mar 2, 2015 at 16:28

3 Answers 3

2

As Frank said, you appear to have figured out a lot of this on your own already, so I only have one piece of advice regarding the main question.

Do the refactoring gradually, incrementally, alongside whatever changes you make to add business value. Don't try to break the 5000 LOC monster into five 1000 LOC critters all in one go. As long as you keep in mind all the long-term goals you listed, the networking code, the JSON parsing, the database handling and whatnot will all steadily inch their way into separate self-contained corners of the codebase.

2
+100

Of course, I never saw your actual code, but from what you've said, I would propose something close to the following architecture. I'm sorry for pseudo-code, I just wanted it to be fast & clear ;)

  1. UI layer:

    dataService = new DataService();
    
    dataService->makeDataRequest(requestName, requestParams[], cachedDataArrivedCallback, freshDataArrivedCallback);
    
    showWaitMessageToUser();
    
  2. Service layer (DataService fictional class):

    processor = RequestProcessorFactory.getProcessor(requestName);
    
    if(processor.cachedDataAvailable(requestParams)) {
        cachedDataCallback(processor.getCachedData());
    }
    
    freshData = processor.getFreshDataAsync(requestParams, freshDataArrivedCallback);
    
  3. RequestProcessor::getFreshDataAsync:

    networkTransport = NetworkRequestFactory.getTransport(requestName);
    networkTransport.send(requestParams[]);
    rawResult = networkTransport.result();
    
    deserializer = DeserializerFactory.getDeserializer(requestName); //JSON deserializers
    result = deserializer.deserialize(rawResult);
    
    storageHandler = StorageHandlerFactory.getStorageHandler(requestName); //Database layer
    storageHandler.storeData(requestName, result);
    
    cacheHandler = CacheHandlerFactory.getCacheHandler(requestName);
    cacheHandler.store(requestParams, result);
    
    freshDataArrivedCallback(result);
    

I don't like diagrams, but I think this should be pretty clear.

Your UI calls your service layer with a request to provide data. It passes there callbacks to execute when data would be available. And then it shows message to user and waits for user interactions.

Service layer first gets RequestProcessor, which is responsible for handling this particular request (User requests, Search requests etc). It checks with processor if it has cached data for this particular request parameters. If so, it gets it and triggers one of UI callbacks. It then asks requestProcessor to go for new data.

Request processor calls network layer to relay request to the server. It deserializes reply using some Deserializer, which is currently your old&ugly JSON library, but you will be able to switch to another pretty easy with what I propose.

Deserialized results go to database handler and cache handler (but may be, they are the same in your app, I am not sure). Well, clean results may go to whatever handlers you want now ;)

Well, at last request processor calls freshDataArrivedCallback to announce that new data is available for UI to display.

This is just an example of how this can be split by. There may be various improvements to the scheme. For example, it is better not to pass freshDataArrivedCallback around, keep it at the DataService and have DataService expose it's own callback to reduce coupling.

This example uses factories every now and again, but you can skip them if you have only one handler for something to avoid unnecessary complications.

I just tried to make my thinking 5-min short to give you an idea ;) I hope you find this useful.

That was an interesting question to answer and if this answer help, I will gladly dive deeper ;) BTW, I never tried to do iOS development ;))

2

I can see some sensible answers to this question already, but I thought I would expand and offer my opinion.

You've described a system that appears to couple logic into large classes, and be tightly coupled across many different functions. The first way to try and refactor this is to first ensure you understand what a 'good' architecture/codebase would look like.

Hopefully you are familiar with SOLID development principles, but if not this is going to really apply here. I will pick a couple of the principles that will ultimately apply to you, but you should ensure you understand them all.

Single Responsibility Principle (SRP - or the S in SOLID) advises ensuring that you create classes that only deal with one thing, and one thing only. Using this principle, you should see code that is focused on individual tasks, and are ignorant to anything outside of their scope. For instance, your networking code should not have anything to do with JSON parsing, as that is a separate responsibility entirely.

Dependency Inversion (DI - of the D in SOLID) states that classes should 'depend upon abstractions. Do not depend on concretions'. This goes hand in hand with ISP, in that a consuming class should use a dependency on an interface, leaving an IOC container to inject the appropriate concrete class as required. This removes dependencies from your code, and helps enforce SRP, as well as making testing much easier.

Hopefully that gives you an idea of what good code quality consists of. If SOLID is followed in the codebase, you should see classes coding against interfaces, with dependencies injected in. Classes should be singularly focused on their domain, and not performing other tasks.

So with regards to the architecture, you can begin to separate logic into distinct layers, with logic between the layers being handled via service calls. A very common approach to this is the Model-View-Controller architecture, which separates UI into a View, data into a model, and wiring logic into a controller. This is typically supported with a lower 'service' level, which provides specialised functionality, such as JSON parsing, data access etc. The service layer should be separate from the above, and expose an interface, most commonly through a RESTful API.

With the above in place, your example of replacing the JSON parser would now require that you simply write a new concrete class that implements the IJsonParser interface (or whatever it may be) and is wired up through your IOC (Dependency Injection) container. So long as the new class implements all of the logic in the interface, then that should be the only change required.

I think the second question warrants a seperate question on this thread, as it could potentially lead to some confusion in this answer, and certainly has a lot of options to it.

1
  • well, I would love to hear an opinion, anyway - for the second question
    – enum
    Mar 6, 2015 at 12:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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