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

Unmaintainable Code - iOS Developer Perspective

Unmaintainable Code - iOS Developer Perspective

Is code duplication the root of all evil? Should I always write a generic code? How to make others life harder?

A subjective journey through common iOS anti-patterns, not so best practices and code smells from the perspective of an experienced Software Engineer.

Based on ~10 years of commercial experience, thousands of code reviews, made with a dose of pragmatism.

Tomasz Gebarowski

October 06, 2017
Tweet

More Decks by Tomasz Gebarowski

Other Decks in Programming

Transcript

  1. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 7 people are

    lazy juniors are afraid of seniors they think that they could be wrong people tend to ✅ auto-accept already reviewed code POOR QUALITY OF REVIEW PROCESS
  2. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 8 we start

    from trivial things learn from our errors and past decisions take your time use checklists HOW TO MASTER CODE REVIEWS?
  3. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 9 we start

    from trivial things learn from our errors and past decisions take your time use checklists HOW TO MASTER CODE REVIEWS?
  4. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 12 a code

    smell is a surface indication that usually corresponds to a deeper problem in the system MARTIN FOWLER
  5. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 14 Method Level

    Class Level Application Level Too many parameters Too short name Ambiguous names Excessive return of data SRP violation Cyclomatic complexity God line Excessively short identifiers Partial methods
  6. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 15 Method Level

    Class Level Application Level Too many parameters Too short name Ambiguous names Excessive return of data SRP violation Cyclomatic complexity God line Excessively short identifiers lint SonarQube CodeBeat
  7. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 16 Class Level

    Method Level Application Level Data clump Feature envy Excessive use of literals Downcasting Orphan variable Inappropriate intimacy Divergent change Strange side effects Refused Bequest Switch statement
  8. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 17 Application Level

    Method Level Class Level Contrived complexity Duplicated code Speculative generality God object Inappropriate design patterns Shotgun surgery Middle man Inappropriate intimacy Parallel Inheritance Hierarchy
  9. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 18 func parse(input:

    String) -> [Result] { } OPTIONALS 1 struct Result { let subject: String? let weight: Int? let mark: Double? }
  10. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 19 struct Result

    { let subject: String? let weight: Int? let mark: Double? } OPTIONALS 1
  11. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 20 struct Result

    { let subject: String? let weight: Int? var mark: Double? { didSet { isGood = (mark ?? 0) >= 4 } } var isGood: Bool = false } OPTIONALS 1
  12. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 21 OPTIONALS 1

    class ResultPrinter { static func printDescription(result: Result) { print("\(result.subject ?? "No Name")") print("mark: \(result.mark ?? 0)") print("weight: \(result.weight ?? 1)") print(" -> \(isGood(result: result))") } private static func isGood(result: Result?) -> String { return result?.isGood ? "good" : "not good" } }
  13. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 22 OPTIONALS func

    average(results: [Result]) -> Double { return Double(results.reduce(0, { return $0 + ($1.mark ?? 0) })) / Double(results.count) } 1
  14. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 23 struct Result

    { let subject: String? let weight: Int? var mark: Double? { didSet { isGood = (mark ?? 0) >= 4 } } var isGood: Bool = false } CODE SMELLS (1) 1 Introducing side effects in setter
  15. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 24 CODE SMELLS

    (2) func average(results: [Result]) -> Double { return Double(results.reduce(0, { return $0 + ($1.mark ?? 0) })) / Double(results.count) } 1 class ResultPrinter { static func printDescription(result: Result) { print("\(result.subject ?? "No Name")") print("mark: \(result.mark ?? 0)") print("weight: \(result.weight ?? 1)") print(" -> \(isGood(result: result))") } ... } Fallbacks + Divergent Change
  16. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 25 CODE SMELLS

    (3) func average(results: [Result]) -> Double { return Double(results.reduce(0, { return $0 + ($1.mark ?? 0) })) / Double(results.count) } 1 class ResultPrinter { static func printDescription(result: Result) { print("\(result.subject ?? "No Name")") print("mark: \(result.mark ?? 0)") print("weight: \(result.weight ?? 1)") print(" -> \(isGood(result: result))") } ... } Feature Envy
  17. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 26 func parse(input:

    String) -> [Result] { } struct Result { let subject: String let mark: Double let weight: Int } IMPROVEMENTS (1) Unwrap early Avoid fallbacks and not consistent default values Do not pass optional when not needed 1
  18. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 27 IMPROVEMENTS (2)

    Eliminate side effects by introducing computed property 1 extension Result { var isGood: Bool { return mark > 4 } } extension Result: CustomStringConvertible { var description: String { return "Subject: \(subject ?? “Unknown”) ...” } } Get rid of Feature Envy class
  19. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 30 open class

    BaseViewController: UIViewController { var disableSwipeBack: Bool = true override open func viewWillAppear(_ animated: Bool) { super.viewWillAppear(animated) if disableSwipeBack { ... } } override open func viewWillDisappear(_ animated: Bool) { super.viewWillDisappear(animated) if disableSwipeBack { ... } } }
  20. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 31 extension BaseViewController

    { func showActivityIndicator() { /* ... */ } func hideActivityIndicator() { /* ... */ } }
  21. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 32 extension BaseViewController

    { func showActivityIndicator() { /* ... */ } func hideActivityIndicator() { /* ... */ } } extension BaseViewController { func handleLeftBarButtonItem() { /* ... */} var leftBarButtonItem: UIBarButtonItem? { return nil /* TODO */ } }
  22. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 33 God object

    Refused Bequest - exposes functionality not needed by all subclasses Override to block certain behaviour BaseViewController is not a good name PITFALLS 2
  23. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 37 open class

    SwipeBackBlocker: UIViewController { override open func viewWillAppear(_ animated: Bool) { super.viewWillAppear(animated) popGestureRecognizer?.isEnabled = false popGestureRecognizer?.delegate = self } override open func viewWillDisappear(_ animated: Bool) { super.viewWillDisappear(animated) popGestureRecognizer?.isEnabled = true popGestureRecognizer?.delegate = nil } private var popGestureRecognizer: UIGestureRecognizer? { return navigationController?. interactivePopGestureRecognizer } }
  24. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 38 public protocol

    SwipeBackBlockerType { func disableSwipeBack() } public extension SwipeBackBlockerType where Self: UIViewController { func disableSwipeBack() { let viewController = SwipeBackBlocker() addChildViewController(viewController) view.addSubview(viewController.view) viewController.view.alpha = 0 viewController.didMove(toParentViewController: self) } }
  25. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 39 class ViewController:

    UIViewController { override func viewDidLoad() { super.viewDidLoad() (self as? SwipeBackBlockerType)?. disableSwipeBack() } } class UserViewController: MyApp.ViewController {} extension UserViewController: SwipeBackBlockerType {}
  26. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 40 extension UserViewController:

    LeftNavigationItemBehaviorType { func handleLeftNavigationItemAction() { print("Execute action") } var leftNavigationItemName: String { return "Name" } } Activity Indicator Search Controller Empty States Right Navigation Bar Item
  27. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 42 We all

    ❤ enums enum ParagraphStyle { case header case body case title } 3
  28. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 43 enum ParagraphStyle

    { case header case body case title var fontSize: CGFloat { switch self { case .header: return 16 case .body: return 14 case .title: return 12 } } } Computed properties 3
  29. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 44 enum ParagraphStyle

    { case header case body case title case footer var fontSize: CGFloat { switch self { case .header: return 16 case .body: return 14 case .title: return 12 case .footer: return 12 } } } 3
  30. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 45 enum ParagraphStyle

    { ... var fontSize: CGFloat { switch self { case .header: return 16 case .body: return 14 case .title: return 12 case .footer: return 12 } } var color: UIColor { switch self { case .header: return .blue case .body: return .black case .title: return .grey case .footer: return .black } } 3
  31. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 46 Violates Open/Closed

    principle Switch statement smell Copy and paste PITFALLS 3
  32. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 47 enum ParagraphStyle

    { struct Appearance { let color: UIColor let fontSize: Int } ... var appearance: Appearance { switch self { case .header: return Appearance(color: .blue, fontSize: 16) ... case .footer: return Appearance(color: .black, fontSize: 8) } } } aggregation SOLUTION 1 3
  33. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 48 protocols SOLUTION

    2 protocol ParagraphStyleType { var color: UIColor { get } var fontSize: CGFloat { get } } struct ParagraphStyle { struct Header: ParagraphStyleType { var color: UIColor = .blue var fontSize: CGFloat = 16 } struct Body: ParagraphStyleType { var color: UIColor = .black var fontSize: CGFloat = 12 } } 3
  34. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 49 SOLUTION 2

    extension ParagraphStyle { struct Footer: ParagraphStyleType { var color: UIColor = .black var fontSize: CGFloat = 8 } } struct ParagraphStyle { struct Header: ParagraphStyleType { var color: UIColor = .blue var fontSize: CGFloat = 16 } struct Body: ParagraphStyleType { var color: UIColor = .black var fontSize: CGFloat = 12 } } 3 protocols
  35. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 50 SOLUTION 2

    struct ParagraphStyle { struct Header: ParagraphStyleType { var color: UIColor = .blue var fontSize: CGFloat = 16 } struct Body: ParagraphStyleType { var color: UIColor = .black var fontSize: CGFloat = 12 } } 3 func load(style: ParagraphStyleType) { print("\(style.color) \(style.fontSize)") } load(style: ParagraphStyle.Footer()) protocols
  36. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 57 contrived complexity

    statically specified constraint values 4 switch smell
  37. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 59 speculative generality

    -> wrong abstraction prone to regressions contrived complexity WHAT WENT WRONG? 4
  38. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 60 Placeholder.xib AvatarPlaceholder.xib

    Custom Class PREFER DUPLICATION OVER WRONG ABSTRACTION ButtonPlaceholder.xib + 4
  39. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 61 5 class

    EventReporter { func send(domain: Events.Domain, scope: Events.Scope, action: Events.Action) { print("domain: \(domain), scope: \(scope) action: \(action))") } }
  40. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 62 5 struct

    Events { enum Domain { case auth case featureA } enum Scope { case menu case button case view } enum Action { case login case logout } }
  41. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 63 5 class

    AuthReporter { private let reporter = EventReporter() func reportUserLoginAction() { reporter.send(domain: .auth, scope: .button, action: .login) } func reportUserLogoutAction() { reporter.send(domain: .auth, scope: .button, action: .logout) } }
  42. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 64 5 class

    AuthReporter { private let reporter = EventReporter() func reportUserLoginAction() { reporter.send(domain: .auth, scope: .button, action: .login) } func reportUserLogoutAction() { reporter.send(domain: .auth, scope: .button, action: .logout) } } Middle Man
  43. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 65 5 class

    AuthReporter { private let reporter = EventReporter() func reportUserLoginAction() { reporter.send(domain: .auth, scope: .button, action: .login) } func reportUserLogoutAction() { reporter.send(domain: .auth, scope: .button, action: .logout) } } Middle Man Shotgun surgery
  44. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 66 5 class

    AuthReporter { private let reporter = EventReporter() func reportUserLoginAction() { reporter.send(domain: .auth, scope: .button, action: .login) } func reportUserLogoutAction() { reporter.send(domain: .auth, scope: .button, action: .logout) } } Open Closed Principle violation struct Events { enum Domain { case auth case featureA } enum Scope { case menu case button case view } enum Action { case login case logout } }
  45. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 67 5 SOLUTION

    protocol EventType { static var domain: String { get } associatedtype S associatedtype A } extension EventType { static var domain: String { return "\(self)" } }
  46. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 68 5 SOLUTION

    protocol EventType { static var domain: String { get } associatedtype S associatedtype A } extension EventType { static var domain: String { return "\(self)" } } struct Auth: EventType { typealias S = Scope typealias A = Action enum Scope: String { case menu case button case view } enum Action: String { case login case logout } }
  47. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 69 5 SOLUTION

    class EventReporter<T: EventType> { func send(scope: T.S, action: T.A) { print("domain: \(T.domain), scope: \(scope) action: \(action))") } }
  48. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 70 5 SOLUTION

    class EventReporter<T: EventType> { func send(scope: T.S, action: T.A) { print("domain: \(T.domain), scope: \(scope) action: \(action))") } } let authReporter = EventReporter<Auth>() authReporter.send(scope: Auth.Scope.menu, action: Auth.Action.login) usage example:
  49. MOBICONF.ORG | 5-6 OCTOBER 2017 #mobiconf2017 @tgebarowski 71 Bloaters Change

    Preventers Dispensables Couplers OO Abusers code smells Comments | Duplicated Code | 
 Dead code | Speculative generality Long names | Long classes | 
 Data clumps | Excessive return of data Divergent change | Shotgun surgery | Parallel Inheritance Hierarchy Middle man | Feature Envy | Inappropriate intimacy | Message chain Switch statement | Refused bequest | Temporary field