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

Unmaintainable code - iOS developer perspective

Unmaintainable code - iOS developer perspective

Slides from presentation at CocoaHeads Łódź (iOS wiOSłuje)

Tomasz Gebarowski

March 30, 2018
Tweet

More Decks by Tomasz Gebarowski

Other Decks in Programming

Transcript

  1. CocoaHeads Łódź | 30 MARCH 2018 @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. CocoaHeads Łódź | 30 MARCH 2018 @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
  3. CocoaHeads Łódź | 30 MARCH 2018 @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
  4. CocoaHeads Łódź | 30 MARCH 2018 @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
  5. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 8 we start

    from trivial things learn from our errors and past decisions take your time use checklists HOW TO MASTER CODE REVIEWS?
  6. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 9 we start

    from trivial things learn from our errors and past decisions take your time use checklists HOW TO MASTER CODE REVIEWS?
  7. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 12 a code

    smell is a surface indication that usually corresponds to a deeper problem in the system MARTIN FOWLER
  8. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 13 code smells

    Method Level Class Level Application Level
  9. CocoaHeads Łódź | 30 MARCH 2018 @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
  10. CocoaHeads Łódź | 30 MARCH 2018 @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
  11. CocoaHeads Łódź | 30 MARCH 2018 @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
  12. CocoaHeads Łódź | 30 MARCH 2018 @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
  13. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 18 func parse(input:

    String) -> [Result] { } OPTIONALS 1 struct Result { let subject: String? let weight: Int? let mark: Double? }
  14. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 18 func parse(input:

    String) -> [Result] { } OPTIONALS 1 struct Result { let subject: String? let weight: Int? let mark: Double? }
  15. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 19 struct Result

    { let subject: String? let weight: Int? let mark: Double? } OPTIONALS 1
  16. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 20 struct Result

    { let subject: String? let weight: Int? var mark: Double? { didSet { isGood = (mark ?? 0) >= 4 } } var isGood: Bool = false } OPTIONALS 1
  17. CocoaHeads Łódź | 30 MARCH 2018 @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" } }
  18. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 22 OPTIONALS func

    average(results: [Result]) -> Double { return Double(results.reduce(0, { return $0 + ($1.mark ?? 0) })) / Double(results.count) } 1
  19. CocoaHeads Łódź | 30 MARCH 2018 @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
  20. CocoaHeads Łódź | 30 MARCH 2018 @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
  21. CocoaHeads Łódź | 30 MARCH 2018 @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
  22. CocoaHeads Łódź | 30 MARCH 2018 @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
  23. CocoaHeads Łódź | 30 MARCH 2018 @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
  24. CocoaHeads Łódź | 30 MARCH 2018 @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
  25. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 28 QUESTION: Have

    you ever used class named BaseViewController? 2
  26. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 29 open class

    BaseViewController: UIViewController { }
  27. CocoaHeads Łódź | 30 MARCH 2018 @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 { ... } } }
  28. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 31 extension BaseViewController

    { func showActivityIndicator() { /* ... */ } func hideActivityIndicator() { /* ... */ } }
  29. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 32 extension BaseViewController

    { func showActivityIndicator() { /* ... */ } func hideActivityIndicator() { /* ... */ } } extension BaseViewController { func handleLeftBarButtonItem() { /* ... */} var leftBarButtonItem: UIBarButtonItem? { return nil /* TODO */ } }
  30. CocoaHeads Łódź | 30 MARCH 2018 @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
  31. CocoaHeads Łódź | 30 MARCH 2018 @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
  32. CocoaHeads Łódź | 30 MARCH 2018 @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
  33. CocoaHeads Łódź | 30 MARCH 2018 @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
  34. CocoaHeads Łódź | 30 MARCH 2018 @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 } }
  35. CocoaHeads Łódź | 30 MARCH 2018 @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) } }
  36. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 39 class ViewController:

    UIViewController { override func viewDidLoad() { super.viewDidLoad() (self as? SwipeBackBlockerType)?. disableSwipeBack() } } class UserViewController: MyApp.ViewController {} extension UserViewController: SwipeBackBlockerType {}
  37. CocoaHeads Łódź | 30 MARCH 2018 @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
  38. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 42 We all

    ❤ enums enum ParagraphStyle { case header case body case title } 3
  39. CocoaHeads Łódź | 30 MARCH 2018 @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
  40. CocoaHeads Łódź | 30 MARCH 2018 @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
  41. CocoaHeads Łódź | 30 MARCH 2018 @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
  42. CocoaHeads Łódź | 30 MARCH 2018 @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
  43. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 46 Violates Open/Closed

    principle Switch statement smell Copy and paste PITFALLS 3
  44. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 46 Violates Open/Closed

    principle Switch statement smell Copy and paste PITFALLS 3
  45. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 46 Violates Open/Closed

    principle Switch statement smell Copy and paste PITFALLS 3
  46. CocoaHeads Łódź | 30 MARCH 2018 @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
  47. CocoaHeads Łódź | 30 MARCH 2018 @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
  48. CocoaHeads Łódź | 30 MARCH 2018 @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
  49. CocoaHeads Łódź | 30 MARCH 2018 @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
  50. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 57 contrived complexity

    statically specified constraint values 4 switch smell
  51. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 59 speculative generality

    -> wrong abstraction prone to regressions contrived complexity WHAT WENT WRONG? 4
  52. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 59 speculative generality

    -> wrong abstraction prone to regressions contrived complexity WHAT WENT WRONG? 4
  53. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 59 speculative generality

    -> wrong abstraction prone to regressions contrived complexity WHAT WENT WRONG? 4
  54. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 59 speculative generality

    -> wrong abstraction prone to regressions contrived complexity WHAT WENT WRONG? 4
  55. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 60 Placeholder.xib AvatarPlaceholder.xib

    Custom Class PREFER DUPLICATION OVER WRONG ABSTRACTION ButtonPlaceholder.xib + 4
  56. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 61 5 class

    EventReporter { func send(domain: Events.Domain, scope: Events.Scope, action: Events.Action) { print("domain: \(domain), scope: \(scope) action: \(action))") } }
  57. CocoaHeads Łódź | 30 MARCH 2018 @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 } }
  58. CocoaHeads Łódź | 30 MARCH 2018 @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) } }
  59. CocoaHeads Łódź | 30 MARCH 2018 @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
  60. CocoaHeads Łódź | 30 MARCH 2018 @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
  61. CocoaHeads Łódź | 30 MARCH 2018 @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 } }
  62. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 67 5 SOLUTION

    protocol EventType { static var domain: String { get } associatedtype S associatedtype A } extension EventType { static var domain: String { return "\(self)" } }
  63. CocoaHeads Łódź | 30 MARCH 2018 @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 } }
  64. CocoaHeads Łódź | 30 MARCH 2018 @tgebarowski 69 5 SOLUTION

    class EventReporter<T: EventType> { func send(scope: T.S, action: T.A) { print("domain: \(T.domain), scope: \(scope) action: \(action))") } }
  65. CocoaHeads Łódź | 30 MARCH 2018 @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:
  66. CocoaHeads Łódź | 30 MARCH 2018 @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