snippetjavaMinor
Android recyclerview filter
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,
In below code, mList is used in
```
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
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
notifyItemInsertedstill 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
As the documentation explains, an
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.
I don't understand this question.
Don't repeat yourself
In the
Similarly, in
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
you could keep a sorted list of the keys of
That is, every time you insert or remove a value in
also insert or remove that value from the sorted list.
You could use a
And then you could pass that to
Another optimization could be to replace the linear search for a prefix of
Use for-each loop
Instead of this:
{
results.add(supersetResults.get(i));
}
}
It's recommended to use a for-each loop:
Conventions
The Android convention is to use a prefix of
this naming should not be used for method parameters like
On the other hand,
- Can I optimize
HashMapmemory by usingArrayMap? Should I use comma separated positions inStringinstead ofIntegerobjects 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.putcalls
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.