iOS Code Review Checklist

To encourage iOS developers to approach code review more thoroughly, we’ve built a simple checklist that should be followed when going through an iOS pull request.

Of course, we do not go through it and add checkmarks on every PR but we use it to remind us from time to time what should we look for when code reviewing. This is also a good start for someone who is new in iOS development or code reviewing in general.

1. Are there any merge conflicts in the PR?

  • If yes – return to the assignee before looking further into code.

2. Did the static analysis check run?

  • Eg. for checking the Swift code style we use Swift Lint

3. Check the architecture

  • Eg. data should not be fetched or processed in presenters or in views
  • Eg. views should not be modified in view models

4. Is the code consistent with the agreed coding guidelines?

5. Are there any new Xcode warnings introduced?

6. Prefer static constants over computed properties

Prefer:

static let c: Int = 1234

Over:

static var d: Int { return 1234 }

 

 

7. Check the single source of truth principle

  • Eg. don’t preserve the same information in multiple places in the code

8. Avoid singletons

  • Use dependency injection instead

9. Look for force unwrapping

  • Force unwrap should be avoided (use in < 1% of the cases)
  • Using ! should always be a red flag!
  • Check if early return from functions (eg. guard) is used

10. Constants

  • Extract constant to a struct

11. Check for magic numbers

  • Avoid hardcoding, avoid magic numbers, if needed extract into a constant
  • If added, magic numbers should be commented

12. Check encapsulation

  • Check if public variable or function can be private or private(set)?
  • If a class won’t be ever instantiated – use an enum instead.

13. Check polymorphism

  • Should we mark the class final?
  • Should we use class or static keyword for a function/variable?
  • Prefer composition with protocols over the inheritance.

14. Check for single responsibility

  • The function should have one purpose! Split it in two if makes sense.
  • The class should not have too many responsibilities – split if needed.
  • Avoid massive view controllers, add managers, helpers, utils instead.

15. Check for the explicit use of self keyword

  • Explicit (unneeded) use of self should be avoided because it makes the capturing semantics of self stand out more in closures, and prevents verbosity elsewhere.

16. Performance check

  • Use strings concatenation with /() instead of +
  • Use isEmpty instead of == nil
  • Use ! instead of == false
  • Instantiate DateFormatter only once
  • Reuse cells
  • Use image caching
  • Use background threads where needed
  • Etc…

17. Error handling

  • Are all the errors cases handled in code? Log the error, notify the user, retry, fail safely…

18. Check security vulnerabilities

  • Are there any user passwords or other secrets stored as plain text?
  • Is there any sensitive data being logged?

19. Check naming

  • Is naming clear and consistent. The naming is important for writing self-documenting code!
  • Boolean values should start with is, can, should, will…
  • When in doubt – use longer names (adds more information) over shorter names (can bring confusion).
  • Use UpperCamelCase for types and protocols, lowerCamelCase for everything else.

20. Check duplicate code

  • Duplicate code is a big no-no and should be extracted and reused!

21. Remove unnecessary code

  • Remove commented code. You can always get it back with git.
  • Remove empty and/or unused variables, functions, imports…
  • No need to add explicit init for structs if using only the default initializer.

22. Memory leaks

  • Closures should use weak self
  • Delegates should be weak
  • Check if unowned is misused
  • Check for any retain cycles

23. Check if all strings are localized

24. Is the code covered with logs?

  • Every important event in the app should be logged.
  • The logs should contain enough information (class, function, parameters, severity).
  • Repeating information can pollute the log.

25. Is the code well documented?

  • Does every public function contain a comment?
  • Does the code contain a comment for every specific part of the code?
  • Does every specific class (manager, service, script…) contain a comment about their purpose?
  • Does every file contain copyright?
  • Should the README be updated?

As mentioned before, we use this checklist as a reminder and a motivation for doing better code reviews. Thus, we are adding bullets to our checklist and improving it over time.