Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Butter knife for view injection. #550

Closed
getsanjeev opened this issue Mar 9, 2017 · 15 comments
Closed

Use Butter knife for view injection. #550

getsanjeev opened this issue Mar 9, 2017 · 15 comments

Comments

@getsanjeev
Copy link
Contributor

Most of the projects are using Butter Knife for view injection. It would be really good if we incorporate it. It allows us to perform injection on arbitrary objects, views and OnClickListeners i.e makes life easier.
Reference : http://jakewharton.github.io/butterknife/

@UrjaPawar
Copy link

@getsanjeev interesting! Since the UI is going to be materialised (or at least a good part of it) let me know if I can work on this after this issue is discussed with @lognaturel and @grzesiek2010

@getsanjeev
Copy link
Contributor Author

@UrjaPawar Let it be reviewed first. :

@lognaturel
Copy link
Member

I think this discussion needs to be coupled with #544. The first question is whether we want to introduce any of these kinds of tools. Then the next one is at what level (full dependency injection vs. view injection... recognizing that they can work together). A valid answer might be "yes but later".

Then there's a specific tool question. I think that for view injection at least AndroidAnnotations needs to be considered as well.

@nribeka @srsudar

@srsudar
Copy link
Contributor

srsudar commented Mar 11, 2017

I've seen AndroidAnnotations but haven't used it. My impression (admittedly based on no experience and limited investigation both now and in the past) is that it can help you eliminate boilerplate, but that you really have to buy in.

In my opinion:

  • Dagger injection will require a large restructure of the code. Dagger-based codebases don't look like regular codebases to me. Maybe as I became more familiar with it I would realize that was incorrect. As the OP mentioned on Introduce Dependency Injection (Dagger) #544, it can lead to cleaner code. I don't think that Dagger alone gives you this, however. It more forces you to think about dependencies (which is good) and then makes you abide by good design patterns. Dependency Injection as a design pattern, even if not via a framework, is a good idea, but I don't think moving to Dagger right now would be a good use of resources.
  • Based on the samples I've seen, apps based on AndroidAnnotations will soon stop looking like a regular Android app. If you look at their example on the homepage they show a before and after code snapshot. I see bad code on the left and maybe, possibly, better code on the right? It doesn't look like something I could easily debug or reason about without being intimately familiar with the framework. I don't know what tests would look like for this. Things like their interface BookmarkClient make me worry that AndroidAnnotations would pull a project toward full buy-in. I really enjoyed using Retrofit and RxJava. Would Android Annotations make it harder to adopt these solutions or explore alternatives?
  • Butterknife is much lighter weight. It doesn't really strike me as a peer to these other two options. It eliminates boilerplate but is only focused on Views. It doesn't require restructuring of the codebase at all, it just changes setup code (at least in the basic usage--there might be some advanced configuration I'm not aware of). At its most basic, it just eliminates calls to findViewById. Here is an example Activity.

Before:

public class MyActivity extends Activity {
  protected TextView textView;

  @Override
  public void onCreate(Bundle bundle) {
    super.onCreate(bundle);
    this.textView = (TextView) findViewById(R.id.textView);
  }
}

After:

public class MyActivity extends Activity {
  @BindView(R.id.textView) TextView textView;

  @Override
  public void onCreate(Bundle bundle) {
    super.onCreate(bundle);
    Butterknife.bind(this);
  }
}

When you have a lot of views, this eliminates a lot of code. I don't know what the implications are for Collect, at least for form entry, where views in the form wouldn't be available at compile time. For the non-dynamic Activities I think it would work well.

I'd say Dagger is too big a change right now, especially with lots of people new to the codebase, but could make sense down the road if people feel it's needed.

Android Annotations is a different approach to writing apps that would also be a big change. Maybe people that have used it can argue for it, but if it were only me I'd be hesitant to adopt it. I'm not familiar with any large projects that use it.

Butterknife is much more lightweight. Most of my projects use it now, and I think if people wanted to introduce it as they made changes to different pieces of code I think it would be fine.

@getsanjeev
Copy link
Contributor Author

@srsudar Perfect answer. In case we go for Butterknife, we need to decide how exactly we go about the change.

@shouryalala
Copy link
Contributor

@getsanjeev @srsudar @lognaturel
I havent used Butterknife before although i have used Dagger before. The thing about these libraries is that although they are lightweight, they carry out most of their implementation during run time. Butterknife uses reflection and aggregates all view elements and calls them together per activity, which is what i gathered from a bit of reading.
Now if we really want to begin to go down this road, we should think about every device that'll be using ODK, and every scenario where it will be used. Trying to simplify the code shouldnt take away from the essence of the app. We need to measure if there is any significant performance dip and weigh it with its pros. I'd say not to go ahead with Butterknife at the moment. As it is, collect is relatively a pretty heavy application already.

@srsudar
Copy link
Contributor

srsudar commented Mar 12, 2017

To the best of my understanding, Dagger 2 (unlike Dagger 1) does most of its work at compile time generating regular Java code. I don't believe that most of what they do is at runtime. It was my understanding that Butterknife takes a similar approach, as described here.

I don't think a performance hit from Butterknife is something to be afraid of.

I am +0 for Butterknife.

@getsanjeev
Copy link
Contributor Author

@lognaturel How should we go about this?

@lognaturel
Copy link
Member

Let's postpone this decision to avoid making too many invasive changes at once. If students want to include something like adding Butterknife to their GSoC project proposal and explain why it would help with the project, that would be great too.

@VisheshVadhera
Copy link
Contributor

Any update on this one? I have used Butterknife in many of my projects and I feel I can be of help here.

@lognaturel
Copy link
Member

@VisheshVadhera Thanks for your interest in this discussion! @shobhitagarwal1612 is working on a refresh to the user experience as part of his Google Summer of Code project. You can check out his project proposal and timeline. He's making some pretty far-reaching changes in that context and I think that while that project is ongoing, it makes sense to limit other changes to the UI code unless he thinks they're the right thing to do in parallel.

Beyond that, I think there's more to discuss before we can be in a position to say yes or no to Butterknife or other related options. @srsudar gave a good overview of different options that have been discussed as possible alternatives here. Another thing I realized might be worth also consideration is the Android Data Binding libs. Again, not something I personally have experience with but I think it's worth considering in what ways it may be more or less advantageous than Butterknife in this context.

Relatedly, we never made much progress on #506 and talking about patterns and code structure at a high level but I think it's very much related to this discussion. If anyone is interested in reviving it, the forum's development topic would now be a better place for it since it's a more open-ended discussion.

@asclepiusgal
Copy link

That proposal looks like an awesome update to the UI @shobhitagarwal1612!

@grzesiek2010
Copy link
Member

grzesiek2010 commented Oct 27, 2017

I think the Android Data Binding would be better. Why do we need the Butterknife? I think in most cases we would use that only to get rid of findViewById and looks like thanks to the Android Data Binding we can do that using even less code than using the Butterknife. Here is a good comparison for those who don't know those tools https://www.youtube.com/watch?v=JsSkhR2rFzI

But looks like Kotlin is the best here. Look here at this example: grzesiek2010@ab9ca5d

@shobhitagarwal1612
Copy link
Contributor

@lognaturel We are using both dagger and butter knife so this issue can be closed too.

@grzesiek2010
Copy link
Member

We already use Butterknife so we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants