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

Checking alert dialog validation

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

Problem

I have set alert dialog on textview's onclicklistener, and there I have added some items using array adapter. Now what I want to do is check if the user selects an item from the alert dialog and clicks on submit. It should submit, otherwise not. But as per my if I select or not it displays message fill the details.

boolean isSetDay = false;
 sp3=(TextView)findViewById(R.id.daydates);
 btnsubmit=(Button)findViewById(R.id.btnreg);

 final String[] items = new String[] {"01","02","03","04","05","06","07","08","09","10","11","12","13","14","15","16","17","18","19","20","21","22","23","24","25","26","27","28","29","30","31"};
 adapter123 = new ArrayAdapter(this,
    android.R.layout.simple_spinner_dropdown_item, items);

 sp3.setOnClickListener(new OnClickListener() {

@Override
public void onClick(View w) {
      new AlertDialog.Builder(RegistrationForm.this)
      .setTitle("Select Day")
      .setAdapter(adapter123, new DialogInterface.OnClickListener() {

        @Override
        public void onClick(DialogInterface dialog, int which) {

            sp3.setText(adapter123.getItem(which).toString());
            isSetDay=true;

          dialog.dismiss();
        }
      }).create().show();
    }
  });
     btnsubmit.setOnClickListener(new OnClickListener()
  {

 @Override
 public void onClick(View v) 
 {
    if(emailtext.getText().toString().trim().length() > 0 &&   passtext.getText().toString().trim().length() > 0 && confirmpass.getText().toString().trim().length() > 0 && username.getText().toString().trim().length() > 0 && firstname.getText().toString().trim().length() > 0 && lastname.getText().toString().trim().length() > 0 && isSetDay==true && isSetMonth==true &&    isSetYear==true && isSetPro==true)
{
isSetDay=false;
isSetMonth=false;
isSetYear=false;
isSetPro=false;
new AttemptLogin().execute();

}

  else
 {
    Toast.makeText(getApplicationContext(), "Fill the details", Toast.LENGTH_LONG).show();
 }
  break;
 }
 });

Solution

Please pay attention to formatting.
The code as you posted is hard to read,
and not pleasant to review.

Most notably, the indentation is inconsistent or sometimes random,
and you have very long horizontal lines forcing readers to scroll.
For example this one:

if(emailtext.getText().toString().trim().length() > 0 &&   passtext.getText().toString().trim().length() > 0 && confirmpass.getText().toString().trim().length() > 0 && username.getText().toString().trim().length() > 0 && firstname.getText().toString().trim().length() > 0 && lastname.getText().toString().trim().length() > 0 && isSetDay==true && isSetMonth==true &&    isSetYear==true && isSetPro==true)


Broken up to multiple lines it becomes a lot easier to read:

if (emailtext.getText().toString().trim().length() > 0
            && passtext.getText().toString().trim().length() > 0
            && confirmpass.getText().toString().trim().length() > 0
            && username.getText().toString().trim().length() > 0
            && firstname.getText().toString().trim().length() > 0
            && lastname.getText().toString().trim().length() > 0
            && isSetDay == true
            && isSetMonth == true
            && isSetYear == true
            && isSetPro == true)


Note that using == true with boolean expressions is really pointless,
as you could use the expressions directly:
if (something) { ... } is exactly the same as
if (something == true) { ... }.
So you could drop all the == true from the above.

When checking if a string is empty, instead of str.length() > 0,
a better way is !str.isEmpty().

To avoid code duplication of .getText().toString().trim().isEmpty() it would be a good idea to add a helper function:

private boolean notEmpty(EditText input) {
    return !input.getText().toString().trim().isEmpty();
}


So that you can rewrite the condition in the if like this:

if (notEmpty(emailtext)
            && notEmpty(passtext)
            && notEmpty(confirmpass)
            && notEmpty(username)
            && notEmpty(firstname)
            && notEmpty(lastname)
            && isSetDay
            && isSetMonth
            && isSetYear
            && isSetPro)


Even better, as @hkj suggested,
the notEmpty method could take varargs:

private boolean notEmpty(EditText ... args) {
    for (EditText input : args) {
        if (input.getText().toString().trim().isEmpty()) {
            return false;
        }
    }
    return true;
}


which will allow you to shorten the condition even more:

if (notEmpty(emailtext, passtext, confirmpass, username, firstname, lastname)
            && isSetDay
            && isSetMonth
            && isSetYear
            && isSetPro)


This line is also too long, and also error-prone to type it:

final String[] items = new String[] {"01","02","03","04","05","06","07","08","09","10","11","12","13","14","15","16","17","18","19","20","21","22","23","24","25","26","27","28","29","30","31"};


It would be better to fill this array programmatically, for example:

final String[] items = new String[31];
    for (int i = 0; i < items.length; ++i) {
        items[i] = String.format("%02d", i + 1);
    }

Code Snippets

if(emailtext.getText().toString().trim().length() > 0 &&   passtext.getText().toString().trim().length() > 0 && confirmpass.getText().toString().trim().length() > 0 && username.getText().toString().trim().length() > 0 && firstname.getText().toString().trim().length() > 0 && lastname.getText().toString().trim().length() > 0 && isSetDay==true && isSetMonth==true &&    isSetYear==true && isSetPro==true)
if (emailtext.getText().toString().trim().length() > 0
            && passtext.getText().toString().trim().length() > 0
            && confirmpass.getText().toString().trim().length() > 0
            && username.getText().toString().trim().length() > 0
            && firstname.getText().toString().trim().length() > 0
            && lastname.getText().toString().trim().length() > 0
            && isSetDay == true
            && isSetMonth == true
            && isSetYear == true
            && isSetPro == true)
private boolean notEmpty(EditText input) {
    return !input.getText().toString().trim().isEmpty();
}
if (notEmpty(emailtext)
            && notEmpty(passtext)
            && notEmpty(confirmpass)
            && notEmpty(username)
            && notEmpty(firstname)
            && notEmpty(lastname)
            && isSetDay
            && isSetMonth
            && isSetYear
            && isSetPro)
private boolean notEmpty(EditText ... args) {
    for (EditText input : args) {
        if (input.getText().toString().trim().isEmpty()) {
            return false;
        }
    }
    return true;
}

Context

StackExchange Code Review Q#77372, answer score: 4

Revisions (0)

No revisions yet.