Skip to main content

I am mainly a C# .NET developer, but Java code seems quite clear for me. Your code seems fine, but it might be slightly improved:

1) catch (Exception e) { response.setData("Exception occurred"); return response; }

  1. Here:

      catch (Exception e) {
         response.setData("Exception occurred");
         return response;
     }
    

You might be interested in logging exception's details somewhere.

  1. Reduce nesting (less "spaghetti", more readable). E.g.:

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

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

else can be removed, as return will skip all the rest of the code

Thus, the code might look like this:

    @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;
       }
        
       user = getUserFromDatabase(authToken);
       if (user == null) 
          // what happens if something fails here?
          user = registerUserInDatabase(authToken);
    
       response.setData(user.getPersonalisedWelcome);
       return response;
    }

I am mainly a C# .NET developer, but Java code seems quite clear for me. Your code seems fine, but it might be slightly improved:

1) catch (Exception e) { response.setData("Exception occurred"); return response; }

You might be interested in logging exception's details somewhere.

  1. Reduce nesting (less "spaghetti", more readable). E.g.:

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

else can be removed, as return will skip all the rest of the code

Thus, the code might look like this:

@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;
   }
    
   user = getUserFromDatabase(authToken);
   if (user == null) 
      // what happens if something fails here?
      user = registerUserInDatabase(authToken);

   response.setData(user.getPersonalisedWelcome);
   return response;
}

I am mainly a C# .NET developer, but Java code seems quite clear for me. Your code seems fine, but it might be slightly improved:

  1. Here:

      catch (Exception e) {
         response.setData("Exception occurred");
         return response;
     }
    

You might be interested in logging exception's details somewhere.

  1. Reduce nesting (less "spaghetti", more readable). E.g.:

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

else can be removed, as return will skip all the rest of the code

Thus, the code might look like this:

    @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;
       }
        
       user = getUserFromDatabase(authToken);
       if (user == null) 
          // what happens if something fails here?
          user = registerUserInDatabase(authToken);
    
       response.setData(user.getPersonalisedWelcome);
       return response;
    }
Source Link
Alexei
  • 1.8k
  • 1
  • 14
  • 34

I am mainly a C# .NET developer, but Java code seems quite clear for me. Your code seems fine, but it might be slightly improved:

1) catch (Exception e) { response.setData("Exception occurred"); return response; }

You might be interested in logging exception's details somewhere.

  1. Reduce nesting (less "spaghetti", more readable). E.g.:

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

else can be removed, as return will skip all the rest of the code

Thus, the code might look like this:

@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;
   }
    
   user = getUserFromDatabase(authToken);
   if (user == null) 
      // what happens if something fails here?
      user = registerUserInDatabase(authToken);

   response.setData(user.getPersonalisedWelcome);
   return response;
}