Problem
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?
Solution
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.