I recently changed from HttpUrlConnection
to the Volley
library for Android networking. Since the time that I wrote that first implementation, I've learned a thing or two about testing and writing clean code. One of the principles that I live by is:
One object should only do one thing
What this means is that any class should have one objective and it should be good at that and not worry about doing other stuff. Any information that the object needs should be passed into it (preferably through some form of dependency injection
).
Naive Implementation
So bringing that into the context of an Android Activity
, the following is an example of a naive implementation that gets data from a webservice.
public class MyActivity extends Activity implements AsyncCallBack {
@Override
public void onCreate(Bundle savedInstanceState) {
new AsyncJsonGetTask(this).execute(Service.GET_APPLES);
}
@Override
public void onBackgroundTaskComplete(Object result) {
Apple[] apples = (Apple[]) result;
// do something with apples
}
}
This was my first implementation. Not so bad I guess. I had an AsyncTask
subclass that takes care of all of the network stuff in the background, then calls the calling activity via the AsyncCallBack
interface that includes the onBackgroundTaskCompleted
method. The actual network library used was figured out by another class. There are obvious drawbacks to this, such as the casting which did cause some issues in case of errors and also that it's not easy to test.
Migrating to Volley
Then Volley
came out and I had planned to move to Dagger
for dependency injection so my implementation became:
public class MyActivity extends Activity implements AsyncCallback {
@Inject RequestQueue mRequestQueue;
@Inject ApplesRequest mRequest;
@Override
public void onCreate(Bundle savedInstanceState) {
mRequest.setCaller(this);
mRequest.setMaxApples(20);
mRequestQueue.add(mRequest);
}
@Override
public void onBackgroundTaskComplete(Object result) {
Apple[] apples = (Apple[]) result;
// do something with apples
}
@Override
public void onStop() {
mRequestQueue.cancelAll(); // Cancel any pending requests when activity stops
}
}
This time I use dependency injection. I use a subclass of the Volley Request
object that is related to one particular service. This removes some of the generics but because of the way callbacks are configured, the casting is still there. However, this can be easily fixed by having different interfaces associated with different Request
objects. Testing of course, is much easier now. You inject a mock RequestQueue
for the test and it simply calls back with mock data instead of calling the actual service.
Note: I also switched to EventBus
to make avoid having to many callback interfaces both in these cases and Fragment-Activity communication.
So it seems problem solved. But the implementation is still tightly coupled. If I were to move to some new library from Volley, there would be a lot of refactoring. Not good.
Complete De-coupling
So coming back to the principle, what is the problem here ? Our activity is doing too much. Not only does it have to display whatever about apples, it has to configure a request, pass it to a RequestQueue
and manage the RequestQueue
. That's too much. What our activity should be doing is asking for apples and showing it. That's all.
The solution is to use a WebServiceManager
class. The activitiy asks this class for information and displays it. (This sample implementation also uses EventBus
instead of interfaces).
public class MyActivity extends Activity {
@Inject WebServiceManager mServiceManager;
@Inject EventBus mEventBus;
@Override
public void onCreate(Bundle savedInstanceState) {
mServiceManager.getApples(); // or getApples(20)
}
public void onEvent(Apples[] apples) { // called when WebServiceManager pushes apples to EventBus
// do something with apples
}
public void onEvent(WebServiceError error) { // called when WebServiceManager pushes an error to EventBus
// do something to show error
}
@Override void onStart() {
mEventBus.register(this); // must register to receive onEvent callbacks
}
@Override
public void onStop() {
mEventBus.unregister(this);
mServiceManager.stop(); // Service manager should stop
}
}
Now our activity class doesn't know anything about what's happening with the WebService. No generics. Testing is even easier, just inject a subclass which overrides the methods called by the activity and push mock data to the bus (or mock error to test error handling). The mock class is literally a couple of method calls. And finally, to change to a different low level implementation, just write a new WebServiceManager implementation and change the default injection to that... Boom! App changed over.
Conclusion
This post was in the context of networking but the same principles hold for any service that you use. Write a wrapper for the service that is used throughout the app and then change the implementation over when you have a better one; inject it in and test away. I use this for ImageLoaders too so I can switch between say Picasso
or the imageloader provided by Volley.
Side Note
The only thing I don't really like about this setup is that I'm not using a wrapper for the EventBus
. This means I can't really change over to a different bus easily. The reason I'm living with this for now is that the only other viable eventbus contender, Otto
uses annotations and