googlearchive / android-FingerprintDialog

Migrated:
https://github.com/android/security
Apache License 2.0
1.38k stars 477 forks source link

FingerprintDialog is complicated. #12

Closed keiji closed 9 years ago

keiji commented 9 years ago

I feel this project is very complicated. I don't think Dagger is necessary for sample project.

Please make this project more simple.

thagikura commented 9 years ago

At the time we created the sample, we decided the sample ought to demonstrate how a real life app needs to be constructed including testability. (I know no test code was pushed to the repo so far though..)

What's your actual suggestion? Are you suggesting removing Dagger dependency?

keiji commented 9 years ago

I suggest removing Dagger and adding Decryption(#10).

Because, I think sample applications are not real life app. It's just sample... Sample applications exist for get understanding how to use the function to as many developers as possible.

I think the most important point of FingerprintManager is close coordination with Android Keystore System. This is great feature!

Supplement: I will respect your decision.

thagikura commented 9 years ago

IMO, how samples should behave is not a simple question that there is no single answer that everyone agrees. In fact I personally prefer samples which look like real apps instead of code snippets. Also Dagger only encapsulates the instantiation of the objects instead of changing how fingerprint APIs are called. I think it doesn't have much difference in understanding how fingerprint API works whether the instantiation is made via Dagger or not. So I'd keep the current structure, but I'll definitely keep your feedback for the future.

As for the decription, it makes sense to demonstrate also the decription. I'm going to incorporate it in the next push.

Thanks for your feedback.

keiji commented 9 years ago

I see. thank you too.