17

I have two RecyclerView.Adapters that are using exactly the same RecyclerView.ViewHolders as inner classes.

I wanted to get rid of code duplication and made these ViewHolders free, separate class so the brand new class now can be used by any RecyclerView.Adapters.

However I faced lots of troubles for example difficulty in accessing the adapter objects. getAdapterPosition() always returns -1.

So I changed my mind and made a super RecyclerView.Adapter class which is extended by those adapters and put the ViewHolder in the superclass so those adapters can use it from subclass.

But I want to know if ViewHolder does have to be an inner class. This makes me annoyed. Please do NOT advice me to combine the Adapter classes, they are completely different as the ViewHolder is just a special viewType that can be appear in any RecyclerView

I am waiting for your better approaches which make me feel better.

Regards.

earthw0rmjim
  • 19,027
  • 9
  • 49
  • 63
Egemen Hamutçu
  • 1,602
  • 3
  • 22
  • 34
  • 1
    Java does not even have real inner classes, just some syntactic sugar for writing same-package classes in the same .java file. If you have had issues with ViewHolders as non-inner classes, please post the problematic code. – laalto Sep 06 '16 at 11:07

3 Answers3

11

ViewHolder can be outside class. Inner class is only a proposition in all tutorials for RecyclerView, it is a better way if your ViewHolder should have access to all Adapter parameters, even those private, but any access or objects relations can be recreated by access methods in Adapter and ViewHolder.

I have created stand alone project with usage of ViewHolder as an outside class, take a look. Link to repository - https://github.com/maciejsikora/outsideviewholder.

I think also the cause of your problem is the fact that in the first code version ViewHolder was an inner class and had access to the properties, after change into an outside class, the code should have been refactored, and in the result all relations between ViewHolder and Adapter should be deeply checked.

Answer for the question is - ViewHolder doesn't have to be inner class, and your problems are caused by invalid code implementation in using ViewHolder as an outside class.

Maciej Sikora
  • 19,374
  • 4
  • 49
  • 50
  • There is a button in my viewHolder implementing OnClickListener. When the listener invoked I need which object's button clicked so I must access the position of the adapter. However, calling getAdapterPosition() returns me -1 so I crashed IndexOutOfBounds exception, probably you will also get -1 in your project. :) – Egemen Hamutçu Sep 06 '16 at 12:45
  • @EgemenHamutçu i am sure not. To access position create getter in Adapter and set Adapter to ViewHolder. – Maciej Sikora Sep 06 '16 at 12:47
7

Actually, No.

First you need to understand that why we need Inner class?

We do need Inner classes where we want that only particular class will have this functionality.Like we have many inner class for many Listeners and Button onClick and many more.

So we use inner class for making things private,short and simple .

You can make this thing(ViewHolder) a separate class.But that will not be efficient,clear(If you make another class it will add an extra class to your project),and effective way.

Mehraj Malik
  • 14,872
  • 15
  • 58
  • 85
  • 1
    Inner classes are "extra classes" too. – laalto Sep 06 '16 at 11:25
  • Inner classes are actually very bad practice because they're "hidden" in your project. It's much "cleaner" and recommended to use the "one class per file" principle. – breakline Sep 06 '16 at 11:44
  • Look at this please : http://stackoverflow.com/questions/11398122/what-are-the-purposes-of-inner-classes – Mehraj Malik Sep 06 '16 at 11:48
  • Inner classes are only useful if you wanna access the parent object's variables and methods directly. Which is bad practice because its tight coupling two classes. If you need to communicate between two classes you either should use interfaces or even better, use an publisher-subscriber architechture so you can easily replace a class when testing or in production etc. – breakline Sep 06 '16 at 11:55
  • I agree,we have many ways do a single thing in programming. This is some what similar,that some people love multi threading and some people have fear of multi threading.These things depends individual to individual. That's why **Programmers** made them as a suggestion not a rule. – Mehraj Malik Sep 06 '16 at 12:03
  • @breakline Tight coupling only is bad if you could re-use the affected class(es) somewhere. I have been using ReclerView for many years but never felt the desire to re-use a ViewHolder in another adapter. I prefer the "works as expected with minimal effort" principle. :) Inner classes are as clean as possible. If not they wouldn't exist. I wouldn't need them if I had multiple inheritance. – The incredible Jan May 14 '21 at 11:26
0

I have always used it as inner. I understand your problem and i dealt with it for a while too and i think that this post has the answer for it. The guy from this answer had problems with the adapters too.

Check it here : https://stackoverflow.com/a/29719632/6634292

Your question is an interesting question ;)

Community
  • 1
  • 1
Filipe Gonçalves
  • 181
  • 3
  • 16