Apps-android-commons: Refactoring: Using Butterknife for Android views and callbacks to fields and methods

[maskaravivek]

In my opinion we should start using Butterknife in the app to make the code look more readable. Atleast support for it can be added and any new views can be referenced using Butterknife. Gradually the rest of the codebase can be refactored to start using it. Am suggesting its usage because of the following reasons:

• will make the code look more readable
• will eliminate a lot of boilerplate code
• its better to perform this exercise before the codebase starts getting bigger

Looking forward to opinions from others as well. @misaochan @nicolas-raoul

[nicolas-raoul]

Interesting framework.
Is the Wikipedia app using it? If not, do you know why?
An independent article about the pros and cons of using it would be welcome, if you know any :-)
Also, do you have experience with it, that you could share with us?
Cheers!

[maskaravivek]

The wikipedia app uses butterknife. I couldn't find an article with its pros and cons but here's an article about how it works.
We have been using this library in one of ur projects with a fairly large codebase(50+ modules) without any issues so far. There is no annotations for some view listeners, so you are being forced to mix ButterKnife style with usual Android one. The issues list on the library page lists some of those listeners. I would be happy to help in integrating the library with the existing codebase.

[nicolas-raoul]

If the Wikipedia app uses it, and you are willing to refactor the app to using it, then sounds good to me :-)
@misaochan What do you think?

[misaochan]

I am personally a bit hesitant because, in my opinion, it adds an extra framework that contributors need to learn before they can start contributing. I guess this could just be a personal preference, as I tend to lean towards using minimal additional frameworks myself.

I'm happy to discuss it further though, especially if there are ways to mitigate the drawbacks.

[maskaravivek]

I think the learning curve for using the library is not steep.

• Already lots of android app use this library so many developers might be familiar with it.
• Contributors can continue using the normal Android way if they are not familiar with the library. Butterknife doesn't make it mandatory to use as the only way for view bindings

I can also contribute to a wiki article for its usage in the commons android app.

[tobias47n9e]

Would this also be the time to tackle some user interface changes?

[misaochan]

Thanks for clarifying, @maskaravivek . I think that if no one else is against it, and you are willing to create the wiki article and make the necessary changes for its integration, I'm good with giving it a go. :)

[misaochan]

@tobias47n9e which interface changes do you have in mind?

[tobias47n9e]

I was thinking of the possible integration of the map view and maybe the campaign view. It might be a good topic for the discussion session at the hackathon as well.

[whym]

Contributors can continue using the normal Android way if they are not familiar with the library. Butterknife doesn't make it mandatory to use as the only way for view bindings

Does this mean that Butterknife-based code and traditional code can coexist in one class? If yes, I think it's okay to introduce it gradually. (Assuming that the library will not make the app size too large.)

[maskaravivek]

@whym Yes it can coexist in one class. After adding support for Butterknife and having a wiki in place contributors can use it if they are familiar with it or use the normal android way. However we could consider recommending its usage as its a beautiful framework.

[veyndan]

One problem with using Butterknife (though minor) is that the fields must be at least package-private, which means the scope is unnecessarily broad in some circumstances.

An alternative that I am currently using in one of my projects is to use the generated view binder found using the data binding. Using this doesn't mean that you have to use any of the features of the data binding library (nor would I advocate for such a thing). The main benefit that I found using the generated view binder is that it is type safe, something that neither Butterknife nor the standard Android findViewById offers i.e. you don't have to cast the findViewById to a subclass of View.

Jake Wharton (the writer of Butterknife) even says that the use of it can be advantageous (source). Here's an article which tells you basically all that you need to know about this approach.

I thought I'd just add this to the discussion as it doesn't get talked about much, but would reduce the boilerplate code even more than that of Butterknife alone, is type safe, and IIRC findViewById traverses the layout at runtime whereas using the view binding aspect of databinding, view processing is done at compilation time.

[whym]

Just noting that #409 is getting closer to a merge-able state.

[maskaravivek]

@misaochan Should I continue with the refactoring of other view binding usages after all the discussion around it?

[misaochan]

@maskaravivek I'm good with that, but I would request that the Butterknife wiki is completed first, so that new contributors will be able to refer to it. :)

[maskaravivek]

@misaochan I have added some basic wiki for it. Looking forward to your comments on it.

[misaochan]

Looks good to me! I made a minor modification. :)

[FelixEder]

@maskaravivek Is this issue being worked on at the moment? If not, could I work on it?

[maskaravivek]

@FelixEder You can take this up. Just pick any fragment/activity that isn't using butterknife and replace it with butterknife. :)

[FelixEder]

@maskaravivek I have started work on this. Could you assign me to this issue?

[FelixEder]

@maskaravivek @nicolas-raoul I have gone through all the fragments/activities and see that most seem to have Butterknife already implemented, at least partially. In some of the classes only local variables are not bound, should they still be bound?

Some of the fragments/activity that I've seen that need Butterknife implemented:

• MainActivity

• WelcomeActivity

In some other fragments/activity, like AchievementActivity for example, has some Butterknife view bindings, while there are still some findViewById left. The ones that are not bound only seem to be local variables though. Should I still add Butterknife view bindings to these local variables or do you only do it for fields?

I just want to make sure I follow the correct code convention when it comes to binding views. :)

[FelixEder]

@nicolas-raoul I wont have time to finish this issue sadly, could you remove the assigned-label so someone else can work on this? :ice_cream:

[nicolas-raoul]

@FelixEder Done, thanks for letting us know! :-)