1
\$\begingroup\$

I'm working on mobile app which doing requests to a server's API. I want to develop module that doing following:

  • get authorization key already exists in previous session
  • connect to server api and read server StatusLine answer code
  • if answer code is 403 then try to re authorize
  • when authorization is passed process server answer body

I implement this with state-machine:

enum ConnectionState {
    checkAnswer, reAuth, processLinks, endProcess
}

And made process in while-loop:

        int authCount = 0;
        HttpResponse httpResponse = null;
        ConnectionState state = ConnectionState.checkAnswer;
        boolean processConnect = true;
        while (processConnect) {
            switch (state) {
                case checkAnswer:
                    httpResponse = client.execute(httpGet);
                    final int serverAnswer = httpResponse.getStatusLine().getStatusCode();
                    if (403 == serverAnswer) {
                        state = ConnectionState.reAuth;
                    } else {
                        state = ConnectionState.processLinks;
                    }
                    break;
                case reAuth:
                    if (++authCount > MAX_AUTH_ATTEMPTS) {
                        state = ConnectionState.endProcess;
                        break;
                    }
                    auth.invalidateAuth();
                    if (!auth.doAuth()) {
                        Utils.log("could not authorize api");
                    } else {
                        state = ConnectionState.checkAnswer;
                    }
                    break;
                case processLinks:
                    if (null == httpResponse)
                        throw new RuntimeException("HttpResponse is null");
                    processLinks(httpResponse, imgLinks);
                    state = ConnectionState.endProcess;
                    break;
                case endProcess:
                    processConnect = false;
                    break;
                default:
                    throw new RuntimeException("Finded impossible state: " + state.toString());
            }
        }

What do you think about such code organization for this task? How to write it more elegantly?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ It is common to name enum elements in capital case such as CHECK_ANSWER. \$\endgroup\$
    – DominikM
    Oct 11 '13 at 4:53
3
\$\begingroup\$

I agree with Ibalazscs that StatePattern makes sense - using a switch to drive changes in behavior is a code smell.

Given that this state machine has only one end state, you could ditch the processConnect boolean, and just check for end state...

ConnectionState state = ConnectionState.checkAnswer;
while(! ConnectionState.endProcess.equals(state) {
  ...
}

You also might want to consider whether ConnectionState should implement an interface to make it extensible. See Item #34 "Emulate extensible enums with interfaces", Effective Java Second Edition, by Joshua Bloch

\$\endgroup\$
1
\$\begingroup\$

I would consider the state pattern. The actual state-dependent code could be handled in the enum.

\$\endgroup\$

Your Answer

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

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