5
\$\begingroup\$

I have an app that gets and posts blog posts to and from a server.

Right now I have a network client object which is passed into a service object. The service object is created and called from an Android Activity (though my question could apply to iOS also). Success and failure callbacks are passed from the Activity to the Service object to the Client object.

I'm starting to think that I'm doing something very wrong here, because

  1. I think this violates MVC, because business logic is being done in the activity.
  2. This is becoming difficult to test due to the callbacks being passed from object to object, and I've read that if code is difficult to test, that is a sign of bad design.

Here is my service class

public class PostService implements Service<Post>{

    NetworkClient client;

    PostService(NetworkClient client){
        this.client = client;
    }

    @Override
    public void getAll(Response.Listener<JSONArray> onSuccess, Response.ErrorListener onError){
        client.getList(endpoint(), onSuccess, onError);
    }

    @Override
    public void get(int id, Response.Listener<JSONObject> onSuccess, Response.ErrorListener onError) {
        client.get(endpointForPostNumber(id), onSuccess, onError);
    }

    @Override
    public void create(Post post, Response.Listener onSuccess, Response.ErrorListener onError) {
        JSONObject params = new JSONObject();
        try {
            params.put(ServiceContracts.PostServiceContract.Fields.TEXT, post.getText());
        } catch (JSONException e) {
            e.printStackTrace();
        }
        client.post(endpoint(), params, onSuccess, onError);
    }

    @Override
    public void update(Post post, int id, Response.Listener onSuccess, Response.ErrorListener onError) {
        JSONObject params = new JSONObject();
        try {
            params.put(ServiceContracts.PostServiceContract.Fields.TEXT, post.getText());
        } catch (JSONException e) {
            e.printStackTrace();
        }
        client.patch(endpointForPostNumber(id), params, onSuccess, onError);
    }

    @Override
    public void delete(int id, Response.Listener onSuccess, Response.ErrorListener onError) {
        client.delete(endpointForPostNumber(id), onSuccess, onError);
    }

    // Endpoints
    private static final String ENDPOINT = "/post/";

    private static String endpoint(){
        return Utils.URLS.SERVER_ADDRESS + ENDPOINT + "/";
    }

    private static String endpointForPostNumber(int id){
        return Utils.URLS.SERVER_ADDRESS + ENDPOINT + String.valueOf(id) + "/";
    }
}

Here is my network client class

public class NetworkClient {

    private RequestQueue requestQueue;

    public NetworkClient(RequestQueue requestQueue){
        this.requestQueue = requestQueue;
    }

    public void getList(String endpoint, Response.Listener<JSONArray> onSuccess, Response.ErrorListener onError) {
        JsonArrayRequest jsArrayRequest;
        if (CurrentUser.isLoggedIn()) {
            jsArrayRequest = new JsonArrayRequest(endpoint, onSuccess, onError){
                @Override
                public Map<String, String> getHeaders() throws AuthFailureError {
                    HashMap<String, String> headers = new HashMap<String, String>();
                    headers.put(ServiceContracts.Headers.AUTHORIZATION, "Token " + CurrentUser.getToken());
                    return headers;
                }
            };
        }
        else {
            jsArrayRequest = new JsonArrayRequest(endpoint, onSuccess, onError);
        }

        requestQueue.add(jsArrayRequest);

    }

    public void get(String endpoint, Response.Listener<JSONObject> onSuccess, Response.ErrorListener onError){
        JsonObjectRequest jsObjRequest;
        if (CurrentUser.isLoggedIn()) {
            jsObjRequest = new JsonObjectRequest(Request.Method.GET, endpoint, null, onSuccess, onError){
                @Override
                public Map<String, String> getHeaders() throws AuthFailureError {
                    HashMap<String, String> headers = new HashMap<String, String>();
                    headers.put(ServiceContracts.Headers.AUTHORIZATION, "Token " + CurrentUser.getToken());
                    return headers;
                }
            };
        }
        else {
            jsObjRequest = new JsonObjectRequest(Request.Method.GET, endpoint, null, onSuccess, onError);
        }

        requestQueue.add(jsObjRequest);
    }

    public void post(String endpoint, JSONObject params, Response.Listener<JSONObject> onSuccess, Response.ErrorListener onError){
        JsonObjectRequest jsObjRequest;
        if (CurrentUser.isLoggedIn()) {
            jsObjRequest = new JsonObjectRequest(Request.Method.PUT, endpoint, params, onSuccess, onError) {
                @Override
                public Map<String, String> getHeaders() throws AuthFailureError {
                    HashMap<String, String> headers = new HashMap<String, String>();
                    headers.put(ServiceContracts.Headers.AUTHORIZATION, "Token " + CurrentUser.getToken());
                    return headers;
                }
            };
        }
        else {
            jsObjRequest = new JsonObjectRequest(Request.Method.PUT, endpoint, params, onSuccess, onError);
        }

        requestQueue.add(jsObjRequest);
    }

    public void patch(String endpoint, JSONObject params, Response.Listener<JSONObject> onSuccess, Response.ErrorListener onError){
        JsonObjectRequest jsObjRequest;
        if (CurrentUser.isLoggedIn()) {
            jsObjRequest = new JsonObjectRequest(Request.Method.PATCH, endpoint, params, onSuccess, onError) {

                @Override
                public Map<String, String> getHeaders() throws AuthFailureError {
                    HashMap<String, String> headers = new HashMap<String, String>();
                    headers.put(ServiceContracts.Headers.AUTHORIZATION, "Token " + CurrentUser.getToken());
                    return headers;
                }
            };
        }
        else {
            jsObjRequest = new JsonObjectRequest(Request.Method.PATCH, endpoint, params, onSuccess, onError);
        }

        requestQueue.add(jsObjRequest);
    }

    public void delete(String endpoint, Response.Listener<JSONObject> onSuccess, Response.ErrorListener onError) {
        JsonObjectRequest jsObjRequest;
        if (CurrentUser.isLoggedIn()) {
            jsObjRequest = new JsonObjectRequest(Request.Method.DELETE, endpoint, null, onSuccess, onError) {
                @Override
                public Map<String, String> getHeaders() throws AuthFailureError {
                    HashMap<String, String> headers = new HashMap<String, String>();
                    headers.put(ServiceContracts.Headers.AUTHORIZATION, "Token " + CurrentUser.getToken());
                    return headers;
                }
            };
        }
        else {
            jsObjRequest = new JsonObjectRequest(Request.Method.DELETE, endpoint, null, onSuccess, onError);
        }

        requestQueue.add(jsObjRequest);
    }
}

The post service gets called from and activity like so

postService.getAll(
                    new Response.Listener<JSONArray>() {

                        @Override
                        public void onResponse(JSONArray response) {
                            ArrayList<Post> posts = new ArrayList<Post>();
                            for (int i = 0; i < response.length(); i++) {
                                JSONObject json = null;
                                try {
                                    json = response.getJSONObject(i);
                                    posts.add(new Post(json));
                                } catch (JSONException e) {
                                    popUpInvalidResponseError();
                                }
                                adapter = new WallAdapter(posts, getApplicationContext());
                                listView.setOnItemClickListener(onPostClickListener);
                                listView.setAdapter(adapter);
                            }
                        }
                    },
                    new Response.ErrorListener() {

                        @Override
                        public void onErrorResponse(VolleyError error) {
                            popUpNetworkError(error);
                        }
                    }
            );

The problem is I don't know how to refrain from passing the callbacks through multiple objects and keeping business logic in the Activity because I can't get access the resources in the callback outside of the callback (in java). If I try it tells me I need to make the variable outside the callback final, and then I can't re-assign it inside the callback. I also need to access UI elements in the onError callback.

Does this code follow best practices for the Network/Service layer of a mobile app? If not how can I fix it?

\$\endgroup\$
  • \$\begingroup\$ What kind of business logic is done inside Activity ? If it's about code inside onResponse method above then you can decouple it with a ViewModel or presenter class. You could also return List<Post> in your callback response success rather than JSONArray \$\endgroup\$ – Jabbar_Jigariyo Jun 14 '18 at 6:58
  • \$\begingroup\$ @Jabbar_Jigariyo to be completely honest, I posted this a year ago so I don't remember. I think promises would have addresses the concerns I had in this post (I didn't know what promises were back then). A viewmodel probably would have helped too. \$\endgroup\$ – Matthew Drill Jun 14 '18 at 18:48

Your Answer

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

Browse other questions tagged or ask your own question.