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

refactor: Remove redundant View castings in Android projects #65

Closed
dr0pdb opened this issue Mar 17, 2018 · 16 comments
Closed

refactor: Remove redundant View castings in Android projects #65

dr0pdb opened this issue Mar 17, 2018 · 16 comments
Assignees
Labels
androidlibrary ODK-X Android Library bug Something isn't working good first issue Good for newcomers Low-Priority

Comments

@dr0pdb
Copy link

dr0pdb commented Mar 17, 2018

Software and hardware versions

Latest code from development branch.

Problem description

Currently there are a lot of instances of redundant View castings in the project.
For eg.

infoPane = (LinearLayout) view.findViewById(R.id.sync_info_pane);

can be replaced by

infoPane = view.findViewById(R.id.sync_info_pane);

They were required before but aren't required now and Android studio suggests removing them.

Steps to reproduce the problem

NA

Expected behavior

They should be removed.

Other information

NA

@jbeorse Views ?
Quick win for first timers may be 😄

@jbeorse
Copy link

jbeorse commented Mar 18, 2018

Oh that is interesting. Does this reduce the amount of lint warnings we will get?

@dr0pdb
Copy link
Author

dr0pdb commented Mar 18, 2018

@jbeorse
Yes i think so. Code inspection shows 65 instances of Redundant type casts in the project. A large number of those are these findViewById castings.
Here is a stackoverflow discussion on this.

@jbeorse
Copy link

jbeorse commented Mar 18, 2018

Our compileVersion is 27, so it does seem like this should work. Good find! I'll mark it as a quick win, certainly contributor friendly. Go ahead and claim it if you like. 😄

@jbeorse
Copy link

jbeorse commented Mar 18, 2018

Your title mentions only Services, but I imagine this is true in Survey, Tables, and Scan as well. I think we should extend it to all 4 apps.

@dr0pdb dr0pdb changed the title refactor: Remove redundant View castings in Services refactor: Remove redundant View castings in Android projects Mar 18, 2018
@dr0pdb
Copy link
Author

dr0pdb commented Mar 18, 2018

@opendatakit-bot claim

@getodk-bot
Copy link

Welcome to Open Data Kit, @srv-twry! We just sent you an invite to collaborate on this repository at https://github.com/opendatakit/opendatakit/invitations. Please accept this invite in order to claim this issue and begin a fun and rewarding experience contributing to Open Data Kit!

Here are some tips to get you off to a good start:

  • Please read the README.md and CONTRIBUTING.md in this repo. Those two documents have much of what you need to get started.
  • Join the ODK developer Slack to get help, chat about this issue, and meet the other developers.
  • Sign up and introduce yourself on the ODK community forum to meet the broader ODK community.

See you on the other side (that is, the pull request side)!

@linl33
Copy link
Member

linl33 commented Mar 19, 2018

Hi @srv-twry thank you for your pull requests!

There are some more redundant casts in androidlibrary, after those are fixed I will close this issue.

@wbrunette wbrunette transferred this issue from getodk/getodk Apr 9, 2019
@wbrunette wbrunette added androidlibrary ODK-X Android Library enhancement New feature or request bug Something isn't working good first issue Good for newcomers and removed enhancement New feature or request labels Jul 29, 2019
@VarunT11
Copy link

@linl33 @wbrunette If this issue is still open, I would like to work on this.

@wbrunette
Copy link
Member

@VarunT11 the issue is still open. I will assign it to you.

@VarunT11
Copy link

@wbrunette I have removed the Redundant View Castings from Android Library in odk-x/androidlibrary#191.
Apart from this, I saw some Redundant Castings in Services also which were being suggested to be removed by Android Studio. So shall I remove those Redundant Castings also?

@linl33
Copy link
Member

linl33 commented Mar 22, 2021

@VarunT11 Yes, please remove the ones in Services as well. Thank you.

@wbrunette
Copy link
Member

@VarunT11 can you check out androidcommon, survey, and tables? Then we can close this issue. Good work!

@VarunT11
Copy link

@wbrunette Yes Sure! I will check these as well.

@VarunT11
Copy link

VarunT11 commented Mar 27, 2021

@wbrunette @linl33 While I was going through the code-base of Android Common for finding the Redundant Castings, there were several instances of suggestion by Android Studio to replace some Anonymous Methods with Lambda Expressions. Same is the case with Survey and Tables also.
For example, consider the following code snippet -

new DialogInterface.OnClickListener() {
              @Override
              public void onClick(DialogInterface dialog, int which) {

              //Some Code

              }
}

The above method was being suggested to be replaced with the following -

(dialog, which) -> {

              //Some Code

            }

This replacement would reduce the number of lint warnings and also make some imports unusable and hence we can remove them too.
So shall I replace these Anonymous Methods with the Lambda Expressions?

@wbrunette
Copy link
Member

Since the issue is specifically for redundant casts, let's stick to that. Also ODK-X still supports Android devices that run Android 7 that don't support Lamdas. I hope the Android compile system would handle it correctly but not worth creating weird bugs we would have to track down later.

@VarunT11
Copy link

PS: @wbrunette @linl33 I have made the PRs. Please review them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
androidlibrary ODK-X Android Library bug Something isn't working good first issue Good for newcomers Low-Priority
Projects
None yet
Development

No branches or pull requests

7 participants