patternjavaMinor
Checking alert dialog validation
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:
Broken up to multiple lines it becomes a lot easier to read:
Note that using
as you could use the expressions directly:
So you could drop all the
When checking if a string is empty, instead of
a better way is
To avoid code duplication of
So that you can rewrite the condition in the
Even better, as @hkj suggested,
the
which will allow you to shorten the condition even more:
This line is also too long, and also error-prone to type it:
It would be better to fill this array programmatically, for example:
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.