Can you read my code?

This is the enhanced version of original talk I gave at MobileOptimized 2017. This version has contribution by Sebastiano Gottardo and was the keynote talk at Droidcon Krakow 2017.

Transcript

1. HEALTH TRACKING APP FIGHTING THE TABOO TOGETHER
2. CAN YOU READ MY CODE? DROIDCON KRAKOW 2017
3. CODE IS READ MORE THAN WRITTEN WHY?
4. GROWING TEAMS WHY?
5. YOU AND GOD REALITY CHECK
6. COMMENTS
7. WHAT TO COMMENT?
8. COMMENTS COMMENT THE WHY NOT THE WHAT "foo() looks like this because it turned out all the other 15 solutions we tried sucked”
9. COMMENTS COMMENT MAGIC CONSTANTS val NUM_THREAD = 8 val MAGIC_CONSTANT = 42
10. COMMENTS COMMENT MAGIC CONSTANTS val NUM_THREAD = 8 // My desk legs * 2 val MAGIC_CONSTANT = 42 // The Answer
11. COMMENTS USE COMMON MARKERS // TODO “We should [add random task] in the future” // HACK “This works but is kinda hacky” // HERE BE DRAGONS
12. COMMENTS USE YOUR OWN ANNOTATIONS @TechDebt(jira = "https://biowink.atlassian.net/browse/ATD-28") class BackupActivity : BaseActivity() { … } /** * The [TechDebt] annotation can be used… */ annotation class TechDebt(val jira: String)
13. COMMENTS DO NOT COMMENT ▸ Constructors, getters, setters… Seriously! ▸ Bad functions names. Fix them! ▸ Everything that can be inferred from the context and the code.
14. COMMENTS FROM YOUR HEART TO YOUR CODE What you actually mean and should write: “Careful! This routine is not thread-safe!” What you might write while panicking or thinking you are funny or helpful: “Sh*t! This thing will blow up the moment 
 we start using threads! Fingers crossed!”
15. COMMENTS FROM YOUR HEAD TO YOUR CODE scrollsSubscription = calendar .scrollEvents() .map { it.scrollState() } .filter { it == OnScrollListener.SCROLL_STATE_IDLE } .skip(5) .subscribe( { scrollsCount++ }, { Timber.e(it, "Scroll events observable died ") } )
16. COMMENTS FROM YOUR HEAD TO YOUR CODE /* To be able to effectively track scroll events we start observing every scroll event we focus on the scroll state we ignore everything that's not [OnScrollListener.SCROLL_STATE_IDLE] we skip the first 5 events because Android fires 5 bogus events on screen loading we increment the counter that we will later use when the user navigates away from the screen. */ scrollsSubscription = calendar .scrollEvents() .map { it.scrollState() } .filter { it == OnScrollListener.SCROLL_STATE_IDLE } .skip(5) .subscribe( { scrollsCount++ }, { Timber.e(it, "Scroll events observable died ") } )
17. COMMENTS FROM YOUR HEAD TO YOUR CODE interface AdsManager { fun hasPrefetchedAd(placement: Placement): Boolean }
18. COMMENTS FROM YOUR HEAD TO YOUR CODE interface AdsManager { /** * This method checks if there is a prefetched ad for the given [placement]. * * @param placement the placement for which to check if a prefetched ad is * available * @return true if a prefetched ad for the given placement is available, * false otherwise */ fun hasPrefetchedAd(placement: Placement): Boolean }
19. VARIABLES
20. VARIABLES THEY DON’T DESERVE A NAME tmp retval i, k, j foo
21. VARIABLES PACK INFORMATION IN THE NAME ▸Is being temporary the only actual quality of tmp? ▸If retval is a list of employees that we are returning, why don’t we call it employees? ▸Can those loop indexes improve the general readability somehow?
22. VARIABLES LOOP INDEXES for members[j] for clubs[i] for towns[k] doSomething()
23. VARIABLES LOOP INDEXES for members[members_i] for clubs[clubs_i] for towns[towns_i] doSomething()
24. VARIABLES LOOP INDEXES for members[i] for clubs[j] for towns[k] doSomething()
25. VARIABLES LOOP INDEXES members[i] clubs[j] towns[k] doSomething() members[j] clubs[i] towns[k] doSomething()
26. VARIABLES LOOP INDEXES members[clubs_i] clubs[towns_i] towns[members_i] doSomething()
27. VARIABLES HELPERS VARIABLES if (pair.left.split('@')[0] == "root") {...} //
28. VARIABLES HELPERS VARIABLES if (pair.left.split('@')[0] == "root") {...} // HOW ABOUT THIS?
29. VARIABLES HELPERS VARIABLES val email = pair.left // TODO [CLUE-1234] Migrate to data class
30. VARIABLES HELPERS VARIABLES val username = email.split('@')[0] //
31. VARIABLES HELPERS VARIABLES fun Email.toUsername() = split(‘@')[0] // val username = email.toUsername() //
32. VARIABLES HELPERS VARIABLES val email = … val username = email.toUsername() if (username == “root”) { … }
33. VARIABLES HELPERS VARIABLES if(request.userId == document.userId) { // […] } else { […] }
34. VARIABLES HELPERS VARIABLES val userOwnsDocument = (request.userId == document.userId) if (userOwnsDocument) […] else […] //
35. VARIABLES HELPERS VARIABLES premiumManager.isFeatureUnlocked( country, feature.type, premiumManager.getActiveProducts( getIdsForActiveProducts(), configuration ), configuration )
36. VARIABLES HELPERS VARIABLES val activeProducts = premiumManager.getActiveProducts( getIdsForActiveProducts(), configuration) premiumManager.isFeatureUnlocked( country, feature.type, activeProducts, configuration)
37. BOOLEANS PACK INFORMATION IN NAMES val readPassword = true
38. BOOLEANS PACK INFORMATION IN NAMES val needsPassword = true val userIsAuthenticated = true
39. BOOLEANS PACK INFORMATION IN NAMES val disableSSL = false // val useSSL = true // ❤
40. FUNCTIONS
41. FUNCTIONS PACK INFORMATION IN THE SIGNATURE fun delay(amount: Int) //
42. FUNCTIONS PACK INFORMATION IN THE SIGNATURE fun delay(amount_seconds: Int) //
43. FUNCTIONS PACK INFORMATION IN THE SIGNATURE typealias Seconds = Int fun delay(amount: Seconds) // ❤
44. FUNCTIONS PACK INFORMATION IN THE SIGNATURE fun createCache(size: Int) //
45. FUNCTIONS PACK INFORMATION IN THE SIGNATURE typealias Megabytes = Int fun createCache(size: Megabytes) // ❤
46. FUNCTIONS SIDE NOTES ‣ Aim for small reusable functions ‣ Aim for pure functions 1. Dependencies IN 2. Result OUT 3. No side effects
47. IF THEN WHAT? THE ORDER if (length >= 10) {…} if (10 <= length) {…}
48. IF THEN WHAT? THE ORDER ‣ Left: the expression being interrogated, the one more likely to change ‣ Right: the expression being compared against, the one that stays the same over time
49. IF THEN WHAT? THE ORDER ‣ Positive case first ‣ Simpler case first ‣ More interesting case first ‣ The case that returns earlier first
50. IF THEN WHAT? TERNARY OPERATOR Given that lines are free When one single line can be replaced with four lines of more easy-to-understand code Then don't be a j*rk! ⚠
51. IF THEN WHAT? TERNARY OPERATOR fun isUserOfAge(user: User?) = user?.age.takeIf { it >= 18 } ?: false
52. IF THEN WHAT? TERNARY OPERATOR fun isUserOfAge(user: User?) = if (user == null) false else user.age >= 18
53. THE USER PROFILE REAL WORLD EXAMPLE
54. THE USER PROFILE USER OBJECT data class User( name: String, lastName: String, email: String)
55. OBTAINING THE USER PROFILE fun getUser() : User THE USER PROFILE
56. fun fetchUser() : User THE USER PROFILE OBTAINING THE USER PROFILE
57. fun downloadUser() : User THE USER PROFILE OBTAINING THE USER PROFILE
58. fun loadUser() : User THE USER PROFILE OBTAINING THE USER PROFILE
59. THE USER PROFILE OBTAINING THE USER PROFILE fun loadUser(callback: Callback<User>) fun loadUser(): Observable<User>
60. THE USER PROFILE OBTAINING THE USER PROFILE fun loadUser(): Observable<User> loadUser() .subscribeOn(Schedulers.io()) […]
61. THE USER PROFILE OBTAINING THE USER PROFILE fun loadUser(): Single<User> loadUser() .subscribeOn(Schedulers.io()) […]
62. NAMING IS HARD YES
63. WRAPPING UP HEALTHY DEV TEAM TIPS ‣ YAGNI and KISS ‣ Keep your codebase small ‣ Create a routine of removing bad code ‣ Keep writing and maintaining your tests
64. WRAPPING UP FURTHER READINGS ‣ The Art of Readable Code ‣ Working effectively with legacy code ‣ The pragmatic programmer ‣ Mini Habits: Smaller Habits, Bigger Results
65. BE BRAVE Ivan Morgillo @hamen Sebastiano Gottardo @rotxed