I've been trying to look for a better way in which a user has up to 5 inputs in which they can search for results in a database.
I have 2 EditText
s in which the user can have alphanumeric values and 3 Spinner
s. As you can see in the code, it's badly designed in the way it consists of if
or else if
statements which is just horrible, but it works.
What would be the best way to avoid long if
or else if
statements and be able to pick and choose what you would like to search for in the database? I know there is the switch
but does that not just take a value and return the result of something on the one case
?
if(tag != "" && name != "" && breed != "Breed" && sex != "Sex"){
cursor = cupboard().withDatabase(sqLiteDatabase).query(Cattle.class)
.withSelection("tag LIKE ? AND name LIKE ? AND breed LIKE ? AND sex LIKE ?", tag, name, breed, sex)
.getCursor();
if(cursor.getCount() < 1){
Toast.makeText(getContext(), "Sorry, No Cattle with those details!", Toast.LENGTH_LONG).show();
}
} else if (tag == "" && name == "" && breed == "Breed"){
cursor = cupboard().withDatabase(sqLiteDatabase).query(Cattle.class)
.withSelection("sex LIKE ?", sex)
.getCursor();
if(cursor.getCount() < 1){
Toast.makeText(getContext(), "Sorry, No Cattle with those details!", Toast.LENGTH_LONG).show();
}
} else if (breed != "Breed"){
cursor = cupboard().withDatabase(sqLiteDatabase).query(Cattle.class)
.withSelection("breed LIKE ?", breed)
.getCursor();
if(cursor.getCount() < 1){
Toast.makeText(getContext(), "Sorry, No Cattle with those details!", Toast.LENGTH_LONG).show();
}
} else if (sex != "Sex"){
cursor = cupboard().withDatabase(sqLiteDatabase).query(Cattle.class)
.withSelection("sex LIKE ?", sex)
.getCursor();
if(cursor.getCount() < 1){
Toast.makeText(getContext(), "Sorry, No Cattle with those details!", Toast.LENGTH_LONG).show();
}
} else if (tag == "" && name == "" && sex == "Sex"){
cursor = cupboard().withDatabase(sqLiteDatabase).query(Cattle.class)
.withSelection("breed LIKE ?", breed)
.getCursor();
if(cursor.getCount() < 1){
Toast.makeText(getContext(), "Sorry, No Cattle with those details!", Toast.LENGTH_LONG).show();
}
}
1 Answer 1
Disclaimer: I'm a C# developer and I haven't written Java in ages.
First of all I'd move this to a separate method, with (String selection, String[] selectionArgs)
as parameters:
cursor = cupboard().withDatabase(sqLiteDatabase).query(Cattle.class)
.withSelection(selection, selectionArgs)
.getCursor();
if(cursor.getCount() < 1){
Toast.makeText(getContext(), "Sorry, No Cattle with those details!", Toast.LENGTH_LONG).show();
You can now call that method in each if
instead of copy-pasting the same code over and over again and only changing a small part.
But I'm mostly baffled by the convoluted logic WRT those if
s and the resulting selection
and selectionArgs
. Are you sure you didn't mean to construct something like this (pseudo-code):
var selectionOptions = new Dictionary<String, String>();
if(breed != "" && breed != "Breed")
{
selectionOptions.Add("breed LIKE ?", breed);
}
if(sex != "" && sex != "Sex")
{
selectionOptions.Add("sex LIKE ?", sex);
}
// same for tag and name
// now construct the selection
var selection = string.Join(" AND ", selectionOptions.Keys);
// pass the selection and its arguments to the method I mentioned above
ThatMethodMentionedAbove(selection, selectionOptions.Values);
The logic being:
- Look at each of the four possible parameters
- Only add a parameter to the query if it has a value and its value is not its own name
That would make sense to me, whereas this kind of logic:
if (tag == "" && name == "" && breed == "Breed"){
// snip
.withSelection("sex LIKE ?", sex)
... just looks... odd. And indeed, very un-maintainable.
-
\$\begingroup\$ Thanks for the help! Even looking at your code has thought me something. Yeah it's complicated the way I wrote it alright and I fell into just coping and pasting the next statement to force it to work almost. The last code example you posted for the
tag
,name
,breed
should have been commented. Breed is aSpinner
which the default value isBreed
. I thought the logic of having theSpinner
equal toBreed
would show they are all default except thesex
value. \$\endgroup\$SmiffyKmc– SmiffyKmc2016年09月01日 11:18:47 +00:00Commented Sep 1, 2016 at 11:18 -
\$\begingroup\$ small note: what you're advertising as Dictionary is named Map in java. of course you also know there's no var and that string.Join is not there in Java. Additional note: Maps don't guarantee traversal order as you want it here. Most maps do it right, but it's not guaranteed :/ (not that Dictionary would guarantee it either) \$\endgroup\$Vogel612– Vogel6122016年09月01日 11:21:26 +00:00Commented Sep 1, 2016 at 11:21
-
\$\begingroup\$ @Vogel612 Yeah, I know it isn't robust code, it's more about the idea, the logic: compile a list of key-value pairs and loop through that list to construct the query. \$\endgroup\$BCdotWEB– BCdotWEB2016年09月01日 11:25:11 +00:00Commented Sep 1, 2016 at 11:25
-
\$\begingroup\$ Yeah even the logic sounds better then the way I tried to do it xD. It's after clicking in my head on how you came across it in C# and how to implement it in my code! I actually really appreciate the help everyone as this is my first job which the app is demoing to a client and I was asked to do it and I don't want to write lazy code :) \$\endgroup\$SmiffyKmc– SmiffyKmc2016年09月01日 11:29:06 +00:00Commented Sep 1, 2016 at 11:29
-
\$\begingroup\$ @BCdotWEB thanks for helping me out. Thanks to your code I was able to make it much cleaner, scalable and I've actually made the search way better thanks to you. \$\endgroup\$SmiffyKmc– SmiffyKmc2016年09月01日 23:43:04 +00:00Commented Sep 1, 2016 at 23:43
Explore related questions
See similar questions with these tags.