Object-oriented-ness got out of control (plus maybe some threading problems)

Question

I don't have much experience with building well-designed object oriented systems, and this time I improvised, which lead to the system not working and not giving me any errors.

Basically in my android app, I have a user profile activity that calls a class that queries the remote database using the user ID, and returns values for user avatar and user name.

Until the class was nested inside the profile activity class it was alright, but I decided to move it out of there and change some other stuff and now when I go to My profile I do not see my avatar and I do not see my user name.

Here is the GetUserData class:

public class GetUserData extends Activity {
    private String currentlyLoggedInUserString;
    SharedPreferences sharedPrefs;
    Editor editor;
    int currentlyLoggedInUser;
    private JSONParser jsonParser = new JSONParser();
    private Configurationz configurationz = new Configurationz();
    private ToastMaker toastMaker = new ToastMaker();
    private static final String TAG_SUCCESS = "success";
    private static final String TAG_USER_AVATAR = "user_avatar";
    private static final String TAG_USER_NAME = "user_name";
    private static final String TAG_USER_EMAIL = "user_email";
    private static final String TAG_USER_SEX = "user_sex";
    private static final String TAG_USER_DATE_REGISTERED = "user_date_registered";
    private static final String TAG_USER_LAST_SEEN = "user_last_seen";
    private static final String TAG_USER_PASSWORD = "user_password";
    private static final String APP_SHARED_PREFS = "asdasd_preferences";
    private String userName;
    private String userEmail;
    private String userSex;
    private String userPassword;
    private String userAvatar;
    private String userDateRegistered;
    private String userLastSeen;

    public String getUserName() {
        return userName;
    }

    public void setUserName(String userName) {
        this.userName = userName;
    }

    public String getUserEmail() {
        return userEmail;
    }

    public void setUserEmail(String userEmail) {
        this.userEmail = userEmail;
    }

    public String getUserSex() {
        return userSex;
    }

    public void setUserSex(String userSex) {
        this.userSex = userSex;
    }

    public String getUserPassword() {
        return userPassword;
    }

    public void setUserPassword(String userPassword) {
        this.userPassword = userPassword;
    }

    public String getUserAvatar() {
        return userAvatar;
    }

    public void setUserAvatar(String userAvatar) {
        this.userAvatar = userAvatar;
    }

    public String getUserDateRegistered() {
        return userDateRegistered;
    }

    public void setUserDateRegistered(String userDateRegistered) {
        this.userDateRegistered = userDateRegistered;
    }

    public String getUserLastSeen() {
        return userLastSeen;
    }

    public void setUserLastSeen(String userLastSeen) {
        this.userLastSeen = userLastSeen;
    }

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        sharedPrefs = getApplicationContext().getSharedPreferences(APP_SHARED_PREFS, Context.MODE_PRIVATE);
        new GetUserDataGetter().execute();
    }

    public class GetUserDataGetter extends AsyncTask<String, String, String> {

        @Override
        protected String doInBackground(String... params) {
            int success;
            try {

                List<NameValuePair> parameters = new ArrayList<NameValuePair>();
                // fix these shitty variables.
                currentlyLoggedInUser = sharedPrefs.getInt("currentLoggedInUserId", 0);
                currentlyLoggedInUserString = Integer.toString(currentlyLoggedInUser);
                parameters.add(new BasicNameValuePair("user_id", currentlyLoggedInUserString));

                final JSONObject json = jsonParser.makeHttpRequest(configurationz.URL_PHP_GET_USER_DATA, "POST", parameters); 
                success = json.getInt(TAG_SUCCESS);
                if (success == 1) {
                    // user data found
                    setUserLastSeen(json.getString(TAG_USER_LAST_SEEN));
                    setUserDateRegistered(json.getString(TAG_USER_DATE_REGISTERED));
                    setUserAvatar(json.getString(TAG_USER_AVATAR));
                    setUserSex(json.getString(TAG_USER_SEX));
                    setUserPassword(json.getString(TAG_USER_PASSWORD));
                    setUserEmail(json.getString(TAG_USER_EMAIL));
                    setUserName(json.getString(TAG_USER_NAME));

                    //return json.getString(TAG_USER_AVATAR);
                    return null;
                } else if (success == 2) {
                    //toast about not being able to connect to db;
                    runOnUiThread(new Runnable() {
                        public void run() {

                            //this might cause some SHIT!!!!!!!!!!!! TEST IT!!!
                            toastMaker.toast(getBaseContext(), configurationz.ERROR_MESSAGES_SIGNUP_DEVICE_UNABLE_TO_TAKE_PHOTOS, configurationz, Toast.LENGTH_LONG);

                        }
                        });


                    setUserLastSeen("");
                    setUserDateRegistered("");
                    setUserAvatar("");
                    setUserSex("");
                    setUserPassword("");
                    setUserEmail("");
                    setUserName("");
                    return null;
                } else {

                }
            } catch (JSONException e) {
                e.printStackTrace();
            }
            return null;
        }

    }
}

and here is the MyProfile class:

public class MyProfile extends ActionBarAndSlidingMenu {
    private TableRow myProfileActionButtonsHolder;
    private TextView tvUserName;
    private ImageButton iUserAvatar;
    private Bitmap iUserAvatarBitmap;
    private String avatarPath;
    private String userName;
    private static final String APP_SHARED_PREFS = "asdasd_preferences";
    SharedPreferences sharedPrefs;
    Editor editor;
    int currentlyLoggedInUser;
    boolean userLoggedInState = false;
    private GetUserData getUserData = new GetUserData();

    public MyProfile() {
        super(R.string.app_name);
    }

    // do a check here whether this is the user themselves or some other user

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        sharedPrefs = getApplicationContext().getSharedPreferences(APP_SHARED_PREFS, Context.MODE_PRIVATE);
        setContentView(R.layout.user_profile);

        // check whether user is logged in, otherwise redirect them to
        // login/signup page
        userLoggedInState = sharedPrefs.getBoolean("userLoggedInState", false);
        if (!userLoggedInState) {
            // start intent to get them out of here.
            // Research whether this step is necessary at all
        }

        // define the view components
        myProfileActionButtonsHolder = (TableRow) findViewById(R.id.userProfileActionButtonsHolder);

        // set avatar image
        iUserAvatar = (ImageButton) findViewById(R.id.iUserAvatar);
        avatarPath = getUserData.getUserAvatar();
        if (avatarPath != "") {
            iUserAvatarBitmap = BitmapFactory.decodeFile(avatarPath);
            iUserAvatar.setImageBitmap(iUserAvatarBitmap);
        } else {
            iUserAvatar.setImageResource(R.drawable.avatar_default_male);
        }

        //set user display name
        userName = getUserData.getUserName();
        tvUserName = (TextView) findViewById(R.id.tvUserName);
        tvUserName.setText(userName);


        // create action buttons fragment with "edit" and "settings" buttons
        getSupportFragmentManager().beginTransaction().replace(R.id.userProfileActionButtonsHolder, new MyProfileActionButtonsFragment()).commit();

    }
}

Answer 1

First, you need to read up on programming in general and proper coding guidelines in particular, as this is a bit of a chaos. As soon as your project becomes more complex, this gets unreadable and undebuggable. Second, you should read up on how Android works.

Here's your problem in a nutshell:

An Activity is not just Android's own version of a class and you can't use it as such. An Activity represents a screen that is displayed to the user. No screen to display? No Activity. Thus, your getUserData Activity should be a regular class and not extend activity.

Now, in MyProfile you just declare a member variable with

private GetUserData getUserData = new GetUserData();

This does nothing and it certainly never runs that class' onCreate. Thus, your task is never executed and all your fields return null.

Here's what to do in a nutshell:

1. Create a class UserDetails that has a constructor that takes the username, etc. plus the getters necessary to get these details. Add nothing else. This is what we call Java's version of a value object.

   public class UserDetails {
       private final String mUsername;
   
       public UserDetails(String username) {
            mUsername = username;
       }
   
       public String getUsername() {
           return mUsername;
       }
   }
   

2. Create an interface called IOnUserDetailsReceivedListener with the method onUserDetailsReceived(UserDetails userDetails). The reason for this is that your download task will take some time. You need to get informed when it's done and that's what we use this interface for. This is called a listener pattern.

   public interface IOnUserDetailsReceivedListener {
       public void onUserDetailsReceived(UserDetails userDetails);
       public void onUserDetailsError();
   }
   

3. Create a class Downloader that contains your AsyncTask and that has a method retrieveUserDetails(); or something. In that method, run the async task to download. When you get the data from the server, fill it into a new UserDetails(...) object and then call listener.onUserDetailsReceived(userDetails).

   public class UserDetailsDownloader {
       private IOnUserDetailsReceivedListener mListener;
   
       public UserDetailsDownloader(IOnUserDetailsReceivedListener listener) {
           mListener = listener;
       }
   
       public void downloadUserDetails() {
           //Execute the async task here. In it's onPostExecute, do       mListener.onUserDetailsReceived(userDetails).
       }
   
       private class DownloaderTask extends AsyncTask<String, Integer, UserDetails> {
           @Override
           protected UserDetails doInBackground(String... params) {
               //Download code
               //In downloading there might go stuff wrong. If so, return null as an easy method without any error handling.
               UserDetails userDetails = new UserDetails("downloadedUsername");
               return userDetails;
           }
   
   
           @Override
           protected void onPostExecute(UserDetails userDetails) {
               if(userDetails == null) {
                   if(mListener != null) {
                       //Something went wrong. Tell the listener.
                       mListener.onUserDetailsError();
                   }               
               } else {
                   if(mListener != null) {
                       //Cool! Lets pass the userDetails to the activity.
                       mListener.onUserDetailsReceiver(userDetails);
                   }
               }
           }   
       }
   }
   

4. Let your activity implements IOnUserDetailsReceivedListener.

   public void UserActivity extends Activity implements IOnUserDetailsReceivedListener {
       private UserDetailsDownloader mUserDetailsDownloader;
   
       public void onCreate(...) {
           mUserDetailsDownloader = new UserDetailsDownloader(this);
           mUserDetailsDownloader.downloadUserDetails();
       }
   
       public void onUserDetailsReceived(UserDetails userDetails) {
           //Yeeh we received user data.
       }
   
       public void onUserDetailsError() {
           //Something went wrong. Tell the user?
       }
   }
   

5. When your task is done, it'll call your Activities onUserDetailsReceived method and pass you the UserDetails value object with which you can then do what you want.

Answer 2

I don't know if this is your only problem or not but too much for a comment. You shouldn't use runOnUiThread() in doInBackground()

 runOnUiThread(new Runnable() {
     public void run() {

     //this might cause some SHIT!!!!!!!!!!!! TEST IT!!!
     toastMaker.toast(getBaseContext(), configurationz.ERROR_MESSAGES_SIGNUP_DEVICE_UNABLE_TO_TAKE_PHOTOS, configurationz, Toast.LENGTH_LONG);

    }
});

this is why AsyncTask has onPostExecute() and its other methods...they all run on the UI Thread except for doInBackground()

Instead of return null, returnsuccessand depending on that value, do what you need to inonPostExecute()`.

Edit

onPostExecute() gets its parameter from what doInBackground() returns which is the third param in your declaration public class GetUserDataGetter extends AsyncTask<String, String, String>. So you can change that param or return a String to onPostExecute() from doInBackground().

AsyncTask Docs