Skip to main content
Tweeted twitter.com/StackCodeReview/status/677871734716895233
edited tags; edited title; edited tags
Link
200_success
  • 145.6k
  • 22
  • 191
  • 481

Which OOP design pattern(s) could be used to refactor this API method? to validate Facebook OAuth token

Source Link

Which OOP design pattern(s) could be used to refactor this method?

I am new to Java / OOP and I'm concerned that I have a method which is doing far too much "stuff" - but I don't easily see how it can be shortened in a way which is not contrived / arbitrary.

This is a server-side API method built using Google App Engine.

Currently, the method (which is really just a template at this stage):

  1. Validates a user's Facebook OAuth token against Facebook's own servers - this is all done behind the scenes in a FacebookHelper class I have written.
  2. Catches any exceptions (most likely an IOException) thrown during this validation process.
  3. Asks the user to log in again if their token is invalid (if (authToken == null)).
  4. Else if authentication has been successful, tries to get the user record from my database.
  5. If it doesn't exist, adds the user to the database. (registerUserInDatabase).
  6. Responds with a personalised welcome to the user - after having registered the user if they were not already registered.

Code:

@ApiMethod(name = "getUserData", path = "get_user_data")
public Bean getUserData(@Named("token") String token) {

    Bean response = new Bean();
    FacebookAuthToken authToken;
    User user;
    String userPersonalisedWelcome;

    try {
        authToken = ServerFacebookHelper.getAuthToken(token);
    } catch (Exception e) {
        response.setData("Exception occurred");
        return response;
    }

    if (authToken == null) {
        response.setData("Token invalid, please log in");
        return response;
    } else {
        user = getUserFromDatabase(authToken);
        if (user == null) {
            user = registerUserInDatabase(authToken);
            userPersonalisedWelcome = user.getPersonalisedWelcome;
            response.setData(userPersonalisedWelcome);
        } else {
            response.setData(user.getPersonalisedWelcome);
        }
        return response;
    }
}

Obviously there is already a huge amount of work being done outside this method, such as API calls and database reads/writes, but it still feels far too procedural and frankly "dumb". At the same time, everything it encapsulates is required in order to service this "getUserData" API request - so perhaps I am overthinking here.