HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Android app for sending username and password requests

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
androidappsendingusernamepasswordforrequestsand

Problem

I'm developing an Android app that performs several requests to a server using the AndroidAsynchronousHttpClient. One of these requests (as an example) is responsible to send the username and password. I created a class Sign that has a static method (see below) to which I provide a callback to handle the server's response. Is this considered a bad practice?

I'm asking this because I'm having a hard time testing with Mockito.

public class Sign {

public static void signin(String username, String password,
        final SigninResponseHandler responseHandler) {

    RequestParams params = new RequestParams();
    params.put("username", username);
    params.put("password", password);

    MyAPIClient.getInstance().post(path, params,
            new JsonHttpResponseHandler()
            {
                @Override
                public void onSuccess(JSONObject responseObject)
                {
                    String responseAPIStatus = responseObject.getString("status");
                    if (status.isEquals("success")
                    {
                        responseHandler.callback(true, null);
                    }

                }

                @Override
                public void onFailure(Throwable e, JSONObject errorResponse)
                {
                    responseHandler.callback(false, new CustomError(e));
                }
            });
}

Solution

Overdoing static is a code smell. Basically you have to balance things.

Too much static code typically means not enough code in the objects, or not much object oriented structure. Since objects provide boundaries between the in-object code and the rest of the system, they allow "leverage points" for future change (and for testing). You can, for example, subclass an object to provide a variety of implementations of a particular class method.

With static methods, no such polymorphisim is possible. This can lead to switch statements embedded within the static method (to function on key fields, acting somewhat like the state pattern). The primary reason this tends to become a smell is that switch statements put the burden of code maintenance outside of the object. In extreme cases, it begins to feel like the object is not encapsulating it's behavior, but rather the switch statement dictates external behavior depending on the data fields within the object. Such code becomes difficult to maintain over time.

I would avoid static methods as much as reasonably possible; however, in some cases it is a better choice to use them. For example, one would not typically benefit from subclassing the java core types. So if I needed to write a method that operated on String objects, I would probably prefer to put that as a static method on a StringUtils class. On the other hand, if it were a class that I had written, I would probably go out of my way to stay more object-oriented.

In your example...

MyAPIClient.getInstance().post(path, params,
            new JsonHttpResponseHandler()
            {
                @Override
                public void onSuccess(JSONObject responseObject)
                {
                    String responseAPIStatus = responseObject.getString("status");
                    if (status.isEquals("success")
                    {
                        responseHandler.callback(true, null);
                    }

                }

                @Override
                public void onFailure(Throwable e, JSONObject errorResponse)
                {
                    responseHandler.callback(false, new CustomError(e));
                }
            });


is going to be very hard to test. If only you had made an interface on the MyAPIClient, you could do something like.

public SignIn(String username, String password,
    final SigninResponseHandler responseHandler,
    final MyAPIClient) {
   ...
}


which would make SignIn easily be unit testable, like so

SignIn signIn = new SignIn("bob", "supersecret", new JsonHttpResponseHandler() {...},
    new MockAPI() {... });
signIn.perform();


while the mainline code would look like

SignIn signIn = new SignIn("bob", "supersecret", new JsonHttpResponseHandler() {...},
    MyAPIClient.getInstance());
signIn.perform();

Code Snippets

MyAPIClient.getInstance().post(path, params,
            new JsonHttpResponseHandler()
            {
                @Override
                public void onSuccess(JSONObject responseObject)
                {
                    String responseAPIStatus = responseObject.getString("status");
                    if (status.isEquals("success")
                    {
                        responseHandler.callback(true, null);
                    }

                }

                @Override
                public void onFailure(Throwable e, JSONObject errorResponse)
                {
                    responseHandler.callback(false, new CustomError(e));
                }
            });
public SignIn(String username, String password,
    final SigninResponseHandler responseHandler,
    final MyAPIClient) {
   ...
}
SignIn signIn = new SignIn("bob", "supersecret", new JsonHttpResponseHandler() {...},
    new MockAPI() {... });
signIn.perform();
SignIn signIn = new SignIn("bob", "supersecret", new JsonHttpResponseHandler() {...},
    MyAPIClient.getInstance());
signIn.perform();

Context

StackExchange Code Review Q#20994, answer score: 3

Revisions (0)

No revisions yet.