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

Search filters and sorting filters

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

Problem

In my Android application, I have two types of filters: search filters and sorting filters.

I need to read filters from view, display filter on view, and save them into and from memory. I should include these needed features into one class to help myself add new filters in the future. I also need to divide filters by their type.

Filters in my app represented via:

  • Comparator interface from Java SDK (for sorting filters)



-
SearchCriterion interface (for search filters)

public interface SearchCriterion {
    boolean meetCriterion(T obj);
}


To do something needed I wrote an interface:

public interface Filter {
    void readFrom(View view);
    void displayOn(View view);
    void restoreFrom(SharedPreferences sharedPreferences);
    void saveInto(SharedPreferences.Editor editor);
    void includeInIfNeed(Filters filters);
}


Example of Filter subclass:

```
class SearchFilterByDiscountType implements Filter {

private static class KeysOfDiscountTypes {
public static final String BONUS = getClassName() + "Bonus";
public static final String DISCOUNT = getClassName() + "Discount";
public static final String CASH_BACK = getClassName() + "Cash Back";
}

private static String getClassName() {
return SearchFilterDiscountType.class.getSimpleName();
}

private static final boolean DEFAULT_BONUS = true;
private static final boolean DEFAULT_DISCOUNT = true;
private static final boolean DEFAULT_CASH_BACK = true;

private boolean bonus;
private boolean discount;
private boolean cashBack;

public SearchFilterDiscountType() {
bonus = DEFAULT_BONUS;
discount = DEFAULT_DISCOUNT;
cashBack = DEFAULT_CASH_BACK;
}

@Override
public void readFrom(View view) {
bonus = findCheckBox(view, R.id.bonusCheckBox).isChecked();
discount = findCheckBox(view, R.id.discountCheckBox).isChecked();
cashBack = findCheckBox(view, R.id.cashBack

Solution

You have presented similar code to this before, and while some of the recommendations I had have been implemented, others have not... ;-) This is fine, presumably you have your reasons, but the one that sticks out is the public void includeInIfNeed(Filters filters) method, which is redundant....

As for naming, I have a few recommendations:

  • Filter is a fine name for it. No need to change.



  • SearchFilterByDiscountType does not search, it is used to search. I use the convention that a specialization of a class/implementation should (normally) use the base class as a suffix. So, here the Filter should go to the end, and become DiscountTypeFilter. This is somewhat standard, think of List, ArrayList, LinkedList, etc.



  • Filters is not a completely bad name, I agree it could be better, but if this was 'production' code I probably would not change it because the mess in the change logs would not be worth it. On the other hand, it is simple enough to change now, and I have a number of suffixes I use.... Set, List, Array, Chain, Pack, Group and some others I forget right now. I use the suffix that most closely represents the structure and data access mechanism of the data in the collection. In this case, I would probably use FilterChain.



There are two other suggestions I have:

  • Filters is not actually Serializable, so why is it declared to be?



  • It woul dbe really convenient if the Filters (FilterChain?) class also implements the Filter interface which would allow you to apply filters in a batched fashion.

Context

StackExchange Code Review Q#39786, answer score: 2

Revisions (0)

No revisions yet.