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

Android recyclerview filter

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

Problem

I want to optimize the below code, I have asked a similar question here.
I know this approach is better than my last code but I feel there are ways to optimize this too.This code is used in almost every project and I am not satisfied with this answer. There are not even using last filter results and traversing whole list every time.

I have three points to ask in this question,

  • Can I optimize HashMap memory by using ArrayMap? Should I use comma separated positions in String instead of Integer objects array or any way to use int primitive array?



  • I am not getting any animation in this result, how to get that? (I am using notifyItemInserted still no animation)



  • How much data should be kept in Hashmap, till 2 characters or it should be according to result list size? I would be glad to know if anything can be done better in this code other than these points.



In below code, mList is used in onBindViewHolder method. copyList always contains all data(no insertion or deletion is done on that).

```
class MyFilter extends Filter {

/**
* 1. check do we have search results available (check map has this key)
* 2. if available, remove all rows and add only those which are value for that key (string)
* 3. else check do we have any key available starting like this, s=har, already available -ha then it can be reused
*
* @param constraint
*/
@Override
protected FilterResults performFiltering(CharSequence constraint) {
//Here you have to implement filtering way
final FilterResults results = new FilterResults();

if (!mSearchMap.containsKey(constraint.toString())) {
String supersetKey = getSupersetIfAvailable(mSearchMap, constraint.toString());
if (supersetKey == null) {
List foundPositions = doFullSearch(copyList, constraint.toString());
mSearchMap.put(constraint.toString(), foundPos

Solution

Your main questions



  • Can I optimize HashMap memory by using ArrayMap? Should I use comma separated positions in String instead of Integer objects array or any way to use int primitive array?




As the documentation explains, an ArrayMap may be more memory-efficient, at the expense of speed, and when using fewer than hundreds of items. As for the values, List is natural and reasonable.



  • I am not getting any animation in this result, how to get that? (I am using notifyItemInserted still no animation)




You haven't shared enough code to debug that, and in any case we review working code on Code Review. If you need debugging help, try Stack Overflow instead, and make sure to read their posting guidelines.



  • How much data should be kept in Hashmap, till 2 characters or it should be according to result list size?




I don't understand this question.

Don't repeat yourself

In the performFiltering method there are several repeated elements:

  • constraint.toString() is used multiple times



  • There are two identical mSearchMap.put calls



Similarly, in doFullSearch you repeatedly call s.toLowerCase().

It would be better to avoid such duplications, by saving repeated values in local variables, or by extracting common code from conditional branches to before or after the conditional statement.

Performance

To avoid the repeated sorting and conversion from set to list in getSupersetIfAvailable,
you could keep a sorted list of the keys of mSearchMap.
That is, every time you insert or remove a value in mSearchMap,
also insert or remove that value from the sorted list.
You could use a TreeSet to accomplish this easily.
And then you could pass that to getSupersetIfAvailable instead of mSearchMap.

Another optimization could be to replace the linear search for a prefix of s with binary search.

Use for-each loop

Instead of this:

for (int i = 0; i < supersetResults.size(); i++) {
    if (list.get(supersetResults.get(i)).getEmpName().toLowerCase().startsWith(lowerS))




{
results.add(supersetResults.get(i));
}
}

It's recommended to use a for-each loop:

for (Integer result : supersetResults) {
    if (list.get(result).getEmpName().toLowerCase().startsWith(lowerS)) {
        results.add(result);
    }
}


Conventions

The Android convention is to use a prefix of m in field names,
this naming should not be used for method parameters like mSearchMap here:

private String getSupersetIfAvailable(Map> mSearchMap, String s) {


On the other hand, copyList seems to be a member, but it is not named accordingly. Use a consistent naming scheme, either prefix all fields with m or none of them.

Code Snippets

for (int i = 0; i < supersetResults.size(); i++) {
    if (list.get(supersetResults.get(i)).getEmpName().toLowerCase().startsWith(lowerS))
for (Integer result : supersetResults) {
    if (list.get(result).getEmpName().toLowerCase().startsWith(lowerS)) {
        results.add(result);
    }
}
private String getSupersetIfAvailable(Map<String, List<Integer>> mSearchMap, String s) {

Context

StackExchange Code Review Q#148333, answer score: 4

Revisions (0)

No revisions yet.