Skip to content
New issue

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

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

Already on GitHub? # to your account

[ENHANCEMENT] Replace "findViewById" by "@BindView" #780

Open
4rthurRousseau opened this issue Oct 26, 2019 · 3 comments
Open

[ENHANCEMENT] Replace "findViewById" by "@BindView" #780

4rthurRousseau opened this issue Oct 26, 2019 · 3 comments

Comments

@4rthurRousseau
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I noticed that you're using Butterknife but there is still some "findViewById" usage here and there, sometimes on classes that already use ButterKnife.

Example from the "CurrencyListViewActivity" class :

@BindView(R.id.listView)
RecyclerView mListview;

[...]

mListview = findViewById(R.id.listView);

Describe the solution you'd like
Take some time to tidy up those classes.

@hafiz703
Copy link
Contributor

I can have a go at this

@RyanMarzec
Copy link
Contributor

I have updated some files containing to this issue, for now I am only working on files within "app/src/main/java/io/github/project_travel_mate/utilities/"

I just have a questions before proceeding with a PR for this section.

  1. Would you prefer BindView variables declared at the top Fragment/Activity class or in a ViewHolder class?
    I noticed in most files in the utilities folder use butterknife in the same format as the example above in the initial post, while for example in DailyQuotesFragment a ViewHolder class is used for the BindView variables.

@Swati4star
Copy link
Member

@RyanMarzec
I'll prefer these to be declared in ViewHolder class.

Sure, create a PR and I'll review the changes.

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

Successfully merging a pull request may close this issue.

5 participants