Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Android Pitfalls and Anti-patterns

Android Pitfalls and Anti-patterns

Presented at,
# GDG DevFest Ahmedabad - November 2016
# MAD (Mumbai Android Developers) Meetup - January 2017

Jigar Brahmbhatt

November 26, 2016
Tweet

More Decks by Jigar Brahmbhatt

Other Decks in Programming

Transcript

  1. I do not read..I’ll just ask someone what’s new on

    Android Android Pitfalls and Anti-patterns Jigar Brahmbhatt I do not read..I’ll just ask someone what’s new on Android I read your blog…it’s fantastic…can you share your source code so I can copy paste ? I don’t know enough Java, but I would love to do Android development Is this already available in Support Library ? Since When? Just making this Context static to get work done! Will remove it later What is AsyncLayoutInflater ? velopment move it later Oh yeah! I know what is abstraction in OOPS! Got full marks in college exam What do you mean by “context is leaking” ? Oh it’s an easy fix! Let me just put a NULL check before it Hey dude, don’t worry about Android lint! bunch of warnings huah! Dead code ! Dead code! Dead code! How to freaking know when my user is online and offline ? PM requirement! android:screenOrientation=“portrait" Yay! doSomething(null, null, true, null, false) Javadoc ? Who’s gonna read? public static String MY_GLOBAL_STRING = “”; ne ? PM requirement! I don’t know enough Java, but I would love to do Android development Is this already available in Support Library ? Since When? Just making this Context static to get work done! Will remove it later Oh yeah! I know what is abstraction in OOPS! Got full marks in college exam What do you mean by “context is leaking” ? Oh it’s an easy fix! Let me just put a NULL check before it Dead code ! Dead code! Dead code! Javadoc ? Who’s gonna read? public static String MY_GLOBAL_STRING = “”; ine ? PM requirement! BOOT_COMPLETED Code reviews ? YAGNI “Don’t Repeat Yourself” What is AsyncLayoutInflater ? d development @jabbar_jigariyo
  2. Objective Conferences are not about teaching specifics, but all about

    inspiring and enlightening At work, one may find a guide or mentor, but at the end everything depends on self-learning 2
  3. Kind of pitfalls! 1.Business / Product 2.UI / UX No

    Material design, iOS look-n-feel, not user-friendly etc 4
  4. Kind of pitfalls! 1.Business / Product 2.UI / UX 3.Team

    / Environment No -> code review, continuous integration, code style etc. 5
  5. Kind of pitfalls! 1.Business / Product 2.UI / UX 3.Team

    / Environment 4.Engineer / Developer Let’s talk about them today! 6
  6. # 1 Language Problem 8 ‣ Android is a framework,

    not language ‣ Excitement for Android without understanding Java (recent grads) ‣ Knowledge of OOPS, but don’t utilize ‣ Poor or lack of understanding related to design patterns
  7. editText.addTextChangedListener(new TextWatcher() {
 @Override
 public void beforeTextChanged(CharSequence s, int start,

    int count, int after) {
 
 }
 
 @Override
 public void onTextChanged(CharSequence s, int start, int before, int count) {
 // process the text, and do something
 }
 
 @Override
 public void afterTextChanged(Editable s) {
 
 }
 }); public abstract class MyBaseTextWatcher implements TextWatcher {
 @Override
 public void beforeTextChanged(CharSequence s, int start, int count, int after) {
 // Do nothing
 }
 
 @Override
 public void onTextChanged(CharSequence s, int start, int before, int count) {
 // Do nothing
 }
 
 @Override
 public void afterTextChanged(Editable s) {
 // Do nothing
 }
 } Example: Utilizing OOPS concepts
  8. public abstract class MyBaseTextWatcher implements TextWatcher {
 @Override
 public void

    beforeTextChanged(CharSequence s, int start, int count, int after) {
 // Do nothing
 }
 
 @Override
 public void onTextChanged(CharSequence s, int start, int before, int count) {
 // Do nothing
 }
 
 @Override
 public void afterTextChanged(Editable s) {
 // Do nothing
 }
 } editText.addTextChangedListener(new MyBaseTextWatcher() {
 @Override
 public void onTextChanged(CharSequence s, int start, int before, int count) {
 // process the text, and do something
 }
 });
  9. public abstract class MyBaseTextWatcher implements TextWatcher {
 @Override
 public void

    beforeTextChanged(CharSequence s, int start, int count, int after) {
 // Do nothing
 }
 
 @Override
 public void onTextChanged(CharSequence s, int start, int before, int count) {
 // Do nothing
 }
 
 @Override
 public void afterTextChanged(Editable s) {
 // Do nothing
 }
 } editText.addTextChangedListener(new MyBaseTextWatcher() {
 @Override
 public void onTextChanged(CharSequence s, int start, int before, int count) {
 // process the text, and do something
 }
 });
  10. Example: Solving a problem with better design pattern public enum

    PaymentType {
 PAYTM, FREECHARGE
 }
 
 public void pay(PaymentType paymentType, String phone) {
 if(paymentType == PaymentType.PAYTM) {
 // process with paytm using phone
 } else {
 // process with freecharge using phone
 }
 }
 
 // calling method
 paymentHelper.pay(PaymentType.PAYTM, "9999900000");
  11. Example: Solving a problem with better design pattern public enum

    PaymentType {
 PAYTM, FREECHARGE
 }
 
 public void pay(PaymentType paymentType, String phone) {
 if(paymentType == PaymentType.PAYTM) {
 // process with paytm using phone
 } else {
 // process with freecharge using phone
 }
 }
 
 // calling method
 paymentHelper.pay(PaymentType.PAYTM, "9999900000");
  12. // if freecharge, email is required
 public void pay(PaymentType paymentType,

    String phone, String email) throws IllegalStateException {
 if(paymentType == PaymentType.PAYTM) {
 // process with paytm using phone
 } else if(!TextUtils.isEmpty(email)){
 // process with freecharge using phone and email
 } else {
 // error state because payment type is // freecharge but email is null or empty
 throw new IllegalStateException("blah blah");
 }
 }
 
 // calling method
 try {
 paymentHelper.pay(PaymentType.PAYTM, "9999900000", null);
 } catch(IllegalStateException e) {
 // catch
 } // calling method
 try {
 paymentHelper.pay(PaymentType.FREECHARGE, "9999900000", “[email protected]”);
 } catch(IllegalStateException e) {
 // catch
 }
  13. // if freecharge, email is required
 public void pay(PaymentType paymentType,

    String phone, String email) throws IllegalStateException {
 if(paymentType == PaymentType.PAYTM) {
 // process with paytm using phone
 } else if(!TextUtils.isEmpty(email)){
 // process with freecharge using phone and email
 } else {
 // error state because payment type is // freecharge but email is null or empty
 throw new IllegalStateException("blah blah");
 }
 }
 
 // calling method
 try {
 paymentHelper.pay(PaymentType.PAYTM, "9999900000", null);
 } catch(IllegalStateException e) {
 // catch
 } // calling method
 try {
 paymentHelper.pay(PaymentType.FREECHARGE, "9999900000", “[email protected]”);
 } catch(IllegalStateException e) {
 // catch
 }
  14. public enum PaymentType {
 PAYTM, FREECHARGE, BLAH, BLAH2.....
 }
 


    public void pay(PaymentType paymentType, String phone, String email, .......) 
 throws IllegalStateException {
 switch (paymentType) {
 case PAYTM:
 // check for phone number
 break;
 case FREECHARGE:
 // check for phone and email, or fail
 break;
 ..........
 ..........
 }
 }
 
 // calling method
 try {
 paymentHelper.pay(PaymentType.PAYTM, "9999900000", null, ......);
 } catch(IllegalStateException e) {
 // catch
 }
  15. public enum PaymentType {
 PAYTM, FREECHARGE, BLAH, BLAH2.....
 }
 


    public void pay(PaymentType paymentType, String phone, String email, .......) 
 throws IllegalStateException {
 switch (paymentType) {
 case PAYTM:
 // check for phone number
 break;
 case FREECHARGE:
 // check for phone and email, or fail
 break;
 ..........
 ..........
 }
 }
 
 // calling method
 try {
 paymentHelper.pay(PaymentType.PAYTM, "9999900000", null, ......);
 } catch(IllegalStateException e) {
 // catch
 }
  16. Strategy Pattern ?? public interface PaymentStrategy {
 void pay(int amount);


    } public class PaytmStrategy implements PaymentStrategy {
 
 final String phone;
 
 public PaytmStrategy(String phone) {
 this.phone = phone;
 }
 
 @Override
 public void pay(int amount) {
 // make payment using phone
 }
 }
  17. Strategy Pattern ?? public interface PaymentStrategy {
 void pay(int amount);


    } public class PaytmStrategy implements PaymentStrategy {
 
 final String phone;
 
 public PaytmStrategy(String phone) {
 this.phone = phone;
 }
 
 @Override
 public void pay(int amount) {
 // make payment using phone
 }
 }
  18. public class FreechargeStrategy implements PaymentStrategy {
 
 final String phone;


    final String email;
 
 public FreechargeStrategy(String phone, String email) {
 this.phone = phone;
 this.email = email;
 }
 
 @Override
 public void pay(int amount) {
 // make payment using phone and email
 }
 } // Calling methods
 new PaytmStrategy("9090909090").pay(125);
 
 new FreechargeStrategy("9090909090", "[email protected]").pay(123);
  19. public class FreechargeStrategy implements PaymentStrategy {
 
 final String phone;


    final String email;
 
 public FreechargeStrategy(String phone, String email) {
 this.phone = phone;
 this.email = email;
 }
 
 @Override
 public void pay(int amount) {
 // make payment using phone and email
 }
 } // Calling methods
 new PaytmStrategy("9090909090").pay(125);
 
 new FreechargeStrategy("9090909090", "[email protected]").pay(123);
  20. public class FreechargeStrategy implements PaymentStrategy {
 
 final String phone;


    final String email;
 
 public FreechargeStrategy(String phone, String email) {
 this.phone = phone;
 this.email = email;
 }
 
 @Override
 public void pay(int amount) {
 // make payment using phone and email
 }
 } // Calling methods
 new PaytmStrategy("9090909090").pay(125);
 
 new FreechargeStrategy("9090909090", "[email protected]").pay(123);
  21. 26 Stack Overflow Tech blogs / Medium Google+ YouTube videos

    Conferences Github Slack groups # 2 Copying without understanding
  22. 28 ‣ FragmentTransaction.commitAllowingStateLoss(); This is dangerous because the commit can

    be lost if the activity needs to later be restored from its state, so this should only be used for cases where it is okay for the UI state to change unexpectedly on the user.
  23. ‣ Old APIs == no bug fixes, no new features

    IF YOU’RE NOT READING..WE’RE NOT HIRING YOU!!!!!! @Haptik
  24. ‣ Old APIs == no bug fixes, no new features

    ‣ Can’t provide useful input if you’re not up-to-date IF YOU’RE NOT READING..WE’RE NOT HIRING YOU!!!!!! @Haptik
  25. ‣ Old APIs == no bug fixes, no new features

    ‣ Can’t provide useful input if not up-to-date ‣ Writing unnecessary code because you don’t know what is already available IF YOU’RE NOT READING..WE’RE NOT HIRING YOU!!!!!! @Haptik
  26. ‣ Old APIs == no bug fixes, no new features

    ‣ Can’t provide useful input if not up-to-date ‣ Writing unnecessary code because you don’t know what is already available if(phone != null && !phone.isEmpty()) { // Do something } IF YOU’RE NOT READING..WE’RE NOT HIRING YOU!!!!!! @Haptik
  27. ‣ Old APIs == no bug fixes, no new features

    ‣ Can’t provide useful input if not up-to-date ‣ Writing unnecessary code because you don’t know what is already available if(phone != null && !phone.isEmpty()) {}
 
 // instead of “android.text.TextUtils"
 if(!TextUtils.isEmpty(phone)) { // phone is ready to use } IF YOU’RE NOT READING..WE’RE NOT HIRING YOU!!!!!! @Haptik
  28. ‣ If you’re sole developer or early stage startup then,

    you are suffering from spaghetti code everywhere # 4 I only care about final product problem!
  29. // Commented out below method! It’s not working out //

    // public void thisMethodIsNotWorking(String phone) {
 // this.phone = phone;
 //
 // if(phone != null && !phone.isEmpty()) {
 // Something is not working
 // }
 //
 // // instead of
 // if(!TextUtils.isEmpty(phone)) {
 // probably this is not right
 // }
 // } ‣ If you’re sole developer or early stage startup then, you are suffering from spaghetti code everywhere
  30. ‣ Too many 3rd party libs! Choose wisely Update cycle

    Deprecation issue (Classic example —> UniversalImageLoader)
  31. ‣ Too many 3rd party libs! Choose wisely Update cycle

    Deprecation issue Bugs (React Native)
  32. ‣ Too many 3rd party libs! Choose wisely Update cycle

    Deprecation issue Bugs (React Native)
  33. ‣ Too many 3rd party libs! Choose wisely Update cycle

    Deprecation issue Bugs Too much utility for your own usage
  34. ‣ Too many 3rd party libs! Choose wisely Update cycle

    Deprecation issue Bugs Too much utility for your own usage Size impact (Realm)
  35. ‣ Too many 3rd party libs! Choose wisely Update cycle

    Deprecation issue Bugs Too much utility for your own usage Size impact (Realm) You MUST understand how it works
  36. # 5 Context Leaking “A small leak will sink a

    great ship.” - Benjamin Franklin
  37. # 5 Context Leaking to having static context anywhere! “A

    small leak will sink a great ship.” - Benjamin Franklin
  38. Leak Canary by square https://github.com/square/leakcanary ‣ 13th most starred Java

    repo on Github ‣ Have it running in one of your app flavor
  39. # 6 Let me check for NULL problem! QA/Lead: Hey,

    there is a crash with “Null Pointer Exception” in Crashlytics! You: Aah, should be an easy fix! Give me a sec!
  40. QA/Lead: Hey, there is a crash with “Null Pointer Exception”

    in Crashlytics! You: Aah, should be an easy fix! Give me a sec! if(view != null) {
 view.doSomething();
 }
  41. ‣ Avoid allowing NULL in your method arguments ‣ Use

    method overloading make more appropriate methods ‣ Provide factory methods ‣ Use power of annotations! @Nullable @NonNull
  42. @Override
 public void onSuccess() {
 if (progressDialog != null) {


    progressDialog.dismiss();
 }
 } @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 progressDialog = new ProgressDialog(this);
 } YOU: WE’RE SAFE! NULL CHECK ADDED One more example!
  43. @Override
 public void onSuccess() {
 if (progressDialog != null) {


    progressDialog.dismiss();
 }
 } @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 progressDialog = new ProgressDialog(this);
 } Fatal Exception: java.lang.IllegalArgumentException: View=com.android.internal.policy.PhoneWindow$DecorView{9c3d915 V.E...... R......D 0,0-1026,348} not attached to window manager at android.view.WindowManagerGlobal.findViewLocked(WindowManagerGlobal.jav a:424) at android.view.WindowManagerGlobal.removeView(WindowManagerGlobal.java: 350) at android.view.WindowManagerImpl.removeViewImmediate(WindowManagerImpl.ja va:116) at android.app.Dialog.dismissDialog(Dialog.java:362) at android.app.Dialog.dismiss(Dialog.java:345)
  44. # 7 Lint - Ignorance is not bliss! ‣ Lint

    is your best friend! (Part QA too) ‣ Tackle literally every lint warning! If you find a false positive, then suppress it!
  45. # 9 APK or AAR size ignorance! ‣ Drawables (most

    unoptimized and ignored area, but highest impact on size) ‣ Not utilizing proguard ‣ Model classes vs direct JSON
  46. # 9 APK or AAR size ignorance! ‣ Model classes

    vs direct JSON public class UserCreationResponse {
 String userId;
 String username;
 boolean error;
 
 public String getUserId() {
 return userId;
 }
 
 public String getUsername() {
 return username;
 }
 
 public boolean isError() {
 return error;
 }
 }
  47. Ending note! ‣ Aim to understand the code you’re adding!

    ‣ Make sure that it is easy to understand and readable ‣ Take some time, and learn something new every week
  48. You MUST aim to read 100% of these websites! https://codelabs.developers.google.com

    https://github.com/googlesamples https://developer.android.com/guide/index.html https://www.youtube.com/user/androiddevelopers/