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

Name filtering in Java

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

Problem

I have a search box for searching for people in a list of names.
Here is the code that runs every time the query changes. I could not find anything open-sourced for this, so I wanted to make sure this looks good and efficient.

How does it look?

private final List mTotalNames = ............ // some ArrayList with all the names
private final List mAvailableNames = new ArrayList(mTotalNames);

private void doOnQueryTextChange(final String s) {
    mAvailableNames.clear();
    if (s.length()  partNameList
                    = new LinkedList(Arrays.asList(name.split("\\s+")));
            if (helper0(query, partNameList)) {
                mAvailableNames.add(user);
            }
        }
    }
    // yeah, out mAvailableNames is all updated
}

private static boolean helper0(final String query, final List partNameList) {
    for (String partQuery : query.split("\\s+")) {
        if (!helper1(partQuery, partNameList)) {
            return false;
        }
    }
    return true;
}

private static boolean helper1(final String partQuery, final List partNameList) {
    if (partQuery.trim().length()  iterator = partNameList.iterator();
    while (iterator.hasNext()) {
        final String partName = iterator.next();
        if (partName.startsWith(partQuery)
                || partName.trim().length() <= 0) {
            iterator.remove();
            return true;
        }
    }
    return false;
}

Solution

Note: The concepts presented in this answer have been implemented in the following 'alternate' implementation in this question: Name Filtering using a Trie

Interesting problem. Your code is hard to read, not because it is unstructured, or messy, but because the helper function names are hard to grasp.... what do they do?

So, on the good side, your 'visual' style is consistent, and neat. All your lines are indented well, and braced, etc. Well, except for this line:

if (partQuery.trim().length() <= 0) return true;


Naming

The first issue is naming..... Four things:

  • helper0 and helper1 .... do not help, despite the name.



  • Not a big deal, it's probably a 'local rule', but it is unconventional to use 'prefixes' in Java for variable names. What does the m mean? mTotalNames should just be totalNames. You are using "Hungarian" notation of some sort, and that is not good practice in Java



  • 'Part' is overloaded in too many places. partQuery, partName, partNameList, it becomes hard to remember which variables are which.



  • iterator is a horrible name. We know it is an iterator, but what does it iterate?



Algorithm

This is a situation where the right data structure will help a lot. You are doing and repeating too much work. Consider this:

  • you convert names to lower case often.



  • you split names.... often.



  • you get prefixes... often.



What you need is a better algorithm, and the right one, is to build a trie.

The trie will be indexed by the characters in the names. This will allow you to search for names in a very fast way, and not do any of the overhead processing repeatedly.
Build the trie once, and then use it repeatedly when you type to filter the names.

Bug

Your code will not process things correctly for a certain subset of names/queries.

Consider the name "Andy Anderson", and the search query "And Andy".... this will not match, because the search query word 'And' will match against 'Andy' first, and then that leaves the search term 'Andy', which will not match against the word 'Anderson'.

You need to sort the search terms in decreasing-length order, and always search with the longest query term first.

Follow Up

This subject intrigues me, I may implement my recommended solution myself... in a day or so.

Code Snippets

if (partQuery.trim().length() <= 0) return true;

Context

StackExchange Code Review Q#75311, answer score: 5

Revisions (0)

No revisions yet.