$30 off During Our Annual Pro Sale. View Details »

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

    View Slide

  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

    View Slide

  3. Kind of pitfalls!
    1.Business / Product
    Notification spamming, tracking user location etc.
    3

    View Slide

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

    View Slide

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

    View Slide

  6. Kind of pitfalls!
    1.Business / Product
    2.UI / UX
    3.Team / Environment
    4.Engineer / Developer
    Let’s talk about them today!
    6

    View Slide

  7. # 1 Language Problem
    7

    View Slide

  8. # 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

    View Slide

  9. 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

    View Slide

  10. 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

    }

    });

    View Slide

  11. 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

    }

    });

    View Slide

  12. 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");

    View Slide

  13. 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");

    View Slide

  14. // 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

    }

    View Slide

  15. // 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

    }

    View Slide

  16. 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

    }

    View Slide

  17. 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

    }

    View Slide

  18. View Slide

  19. Strategy Pattern ??

    View Slide

  20. 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

    }

    }

    View Slide

  21. 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

    }

    }

    View Slide

  22. 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);

    View Slide

  23. 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);

    View Slide

  24. 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);

    View Slide

  25. 25
    # 2 Copying without understanding

    View Slide

  26. 26
    Stack Overflow
    Tech blogs / Medium
    Google+
    YouTube videos
    Conferences
    Github Slack groups
    # 2 Copying without understanding

    View Slide

  27. 27
    ‣ FragmentTransaction.commitAllowingStateLoss();

    View Slide

  28. 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.

    View Slide

  29. 29
    ‣ android:screenOrientation=“portrait” (anti-pattern)

    View Slide

  30. 30
    ‣ android:screenOrientation=“portrait” (anti-pattern)

    View Slide

  31. 31
    ‣ android.intent.action.BOOT_COMPLETED (anti-pattern)

    View Slide

  32. # 3 I don’t read problem!

    View Slide

  33. IF YOU’RE NOT READING..WE’RE NOT HIRING
    YOU!!!!!!
    @Haptik
    # 3 I don’t read problem!

    View Slide

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

    View Slide

  35. ‣ 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

    View Slide

  36. ‣ 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

    View Slide

  37. ‣ 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

    View Slide

  38. ‣ 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

    View Slide

  39. # 4 I only care about final product
    problem!

    View Slide

  40. ‣ 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!

    View Slide

  41. // 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

    View Slide

  42. ‣ Too many 3rd party libs! Choose wisely
    Update cycle

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

  48. ‣ 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

    View Slide

  49. Otherwise,

    View Slide

  50. # 5 Context Leaking
    “A small leak will sink a great ship.” - Benjamin Franklin

    View Slide

  51. # 5 Context Leaking
    to having static context anywhere!
    “A small leak will sink a great ship.” - Benjamin Franklin

    View Slide

  52. HPROF Viewer and Analyzer (Java Heap)
    https://developer.android.com/studio/profile/am-hprof.html

    View Slide

  53. 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

    View Slide

  54. # 6 Aah! Let me check for NULL
    problem!

    View Slide

  55. # 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!

    View Slide

  56. 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();

    }

    View Slide

  57. ‣ Avoid allowing NULL in your method arguments
    ‣ Use method overloading make more appropriate
    methods
    ‣ Provide factory methods
    ‣ Use power of annotations!
    @Nullable @NonNull

    View Slide

  58. @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!

    View Slide

  59. @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)

    View Slide

  60. # 7 Lint - Ignorance is not bliss!

    View Slide

  61. # 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!

    View Slide

  62. # 8 Dead code problem!

    View Slide

  63. # 8 Dead code problem!

    View Slide

  64. # 8 Dead code problem!

    View Slide

  65. # 9 APK or AAR size ignorance!

    View Slide

  66. # 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

    View Slide

  67. # 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;

    }

    }

    View Slide

  68. 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

    View Slide

  69. Thank you
    Twitter: @jabbar_jigariyo

    View Slide

  70. 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/

    View Slide