HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Android login system

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
systemloginandroid

Problem

I am completely unaware of all the Android features and available libraries.

I just designed a login system to practice it. I have referred to a few books and tutorials.

Layout

I am using relative layout. And have two EditText and one button for login and one button to start register activity.

Activity

```
public class LoginActivity extends AppCompatActivity {

private EditText email, password;
ProgressDialog progress;

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_login);

email = (EditText) findViewById(R.id.email);
password = (EditText) findViewById(R.id.password);
progress = new ProgressDialog(this);
}

public void checkLogin(View view){
final String emailText = email.getText().toString().trim();
final String passwordTex = password.getText().toString().trim();

Response.Listener successListener = new Response.Listener() {
@Override
public void onResponse(String response) {
try {
JSONObject jsonResponse = new JSONObject(response);
if(jsonResponse.getInt("status") == 0){
Toast.makeText(getApplicationContext(), jsonResponse.getString("message"), Toast.LENGTH_LONG).show();
Intent intent = new Intent(LoginActivity.this, SubjectActivity.class);
startActivity(intent);
}else if(jsonResponse.getInt("status") == 1 || jsonResponse.getInt("status") == -2){
Toast.makeText(getApplicationContext(), jsonResponse.getString("message"), Toast.LENGTH_LONG).show();
}else{
Toast.makeText(getApplicationContext(), "Serious error.", Toast.LENGTH_LONG).show();
}
}catch(JSONException e){

}
progress.hide();

Solution

-
Variables names are a little misleading - I'd expect email to be a String, not a View. Try emailView or emailTextView, or even better, since they're private mEmailTextView.

-
All the methods in LoginActivity are public. There's very little chance that any of those methods are going to be accessed outside of the instance itself, they can probably be private.

-
Your ProgressDialog is package-private, just because it's default. If you want something package-private, that's fine, but you probably don't (and if you do, you probably want to comment it). Generally, go for as private as possible (private, protected, package-private, public, in that order).

-
You have large blocks of code that aren't super easy to read. Try breaking your code up into bite-sized methods with descriptive names. Java can get very verbose, so modularity and organization goes a long way.

-
There is a library called "Gson" which converts JSON to Java objects that's very nice and clean and eliminates tons of boilerplate.

-
Why is getInstance synchronized?

-
Is there a reason that addToReqQueue is generic? I can't see that it's doing anything (the generic type is not used in the method body).

-
Convention is that most Strings are stored somewhere (either as constants, or in a file, or something). This isn't critical, though.

Context

StackExchange Code Review Q#127265, answer score: 2

Revisions (0)

No revisions yet.