1
\$\begingroup\$

First I have method called in LoginActivity to call my login API. This is my Listener that reads user input and then passes it to a method to call the API:

OnClickListener onSigninClick = new OnClickListener() {


        @Override
        public void onClick(View arg0) {
            String Pass = password.getText().toString();
            String userName = username.getText().toString();
            if (userName.equals("") || Pass.equals("")) {
                Toast.makeText(Login.this,
                        getResources().getString(R.string.enterempty),
                        Toast.LENGTH_LONG).show();
            } else {
                progressD.show();
                userdata user = new userdata();
                user.password = Pass;
                user.username = userName;
                // check network connectivity 
                if (api.isNetworkConnected()) {
                    // api method 
                    get_user_data(user);
                } else {
                  // if there not internet connection 
                  // which better toast or alert dialog 
                    Toast.makeText(Login.this,
                            getResources().getString(R.string.nointernet),
                            Toast.LENGTH_LONG).show();
                }
            }


        }
    };




  // method fot api 
    private void get_user_data(userdata user) {
           // my JSON parser class
            final JsonParsing jsonParser = new JsonParsing();
            Map<String, String> params = new HashMap<String, String>();
            params.put(AppConstants.TAG_username, user.username);
            params.put(AppConstants.TAG_userpassword, user.password);

            String URL = AppConstants.GeneralURL + AppConstants.ACTION + AppConstants.LOGIN;

            JsonObjectRequest jsonObjectRequest = new JsonObjectRequest(URL, new JSONObject(params),
                    new Response.Listener<JSONObject>() {
                        @Override
                        public void onResponse(JSONObject response) {
                            try {
                                String res = response.getString("result");

                                if (res.equals("false")) {
                                    String error = (String) response.get("message");
                                    Toast.makeText(Login.this, error, Toast.LENGTH_LONG).show();
                                } else if (res.equals("true")) {
                                    userdata user=  jsonParser.getUserDataObject((JSONObject) response.get("message"));
    // navigate to main view 
                                }


                            } catch (JSONException e) {
                                e.printStackTrace();
                            }
                        }
                    }, new Response.ErrorListener() {
                @Override
                public void onErrorResponse(VolleyError error) {
                    // handle error
                }
            });

            MySingleton.getInstance(this.getApplicationContext()).getResquestQueue().add(jsonObjectRequest);

        }

My response JSON has two cases:

{"result" : "false" ,"message":"invalid user"} // in fail case

In success credential, user data returned:

{"result" : "true" ,"message":{"user_id" : "60" , .......} }

My parser class:

public class JsonParsing {
    public userdata getUserDataObject(JSONObject jsonobject) {
        userdata user = new userdata();
        try {
            if (jsonobject.has("user_id")) {
                user.userid = jsonobject.getString("user_id");
            }
            if (jsonobject.has("user_mail")) {
                user.email = jsonobject.getString("user_mail");
            }

            if (jsonobject.has("user_age")) {
                user.age = jsonobject.getString("user_age");
            }
            if (jsonobject.has("user_name")) {
                user.username = jsonobject.getString("user_name");
            }
            if (jsonobject.has("user_password")) {
                user.password = jsonobject.getString("user_password");
            }
            if (jsonobject.has("user_photo")) {
                user.userimageURL = jsonobject.getString("user_photo");
            }
            if (jsonobject.has("posts")) {
                JSONArray array = jsonobject.getJSONArray("posts");
                for (int i = 0; i < array.length(); i++)
                    user.questionItems.add(getQuestionData(array
                            .getJSONObject(i)));
            }
        } catch (JSONException e) {
            e.printStackTrace();
        }
        return user;
    }

Could any one review my code? I want to design the class to handle the network response in two case, and parse it if successful. Are there any other tips on reducing my code?

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

Are there any other tips on reducing my code?

By reducing, not sure if you meant shortening. If yes, then you can reduce the duplication in the parser, using a helper method like this:

private String getString(JSONObject jsonobject, String name) {
    return jsonobject.has(name) ? jsonobject.getString(name) : null;
}

Exception handling

Try to limit the scope of try-catch blocks as much as possible, to include only the sensitive part of the code where an exception might happen. I don't have a compiler with me now but it seems to me that you could reduce the scope at some places.

Checking for empty strings

Instead of .equals(""), use the more idiomatic isEmpty().

Naming

The convention for method names is camelCase, don't use snake_case.

Pointless comments

Some of the comments are pointless, noise, for example:

            // check network connectivity 
            if (api.isNetworkConnected()) {

I suggest to remove such comments.

| improve this answer | |
\$\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.