Swift Code Review guide

Swift Code Review guide

Reviewing code can be difficult. Especially when it is code written in Swift. Swift is a very new language and it is in many ways simular to Objective-C and in many (fascinatingly new) ways it is also very different. You can find many style guides on the internet. They served as an inspiration to this one.

Also this is a guide we use to review code. So rather then a style guide we call it a code review guide. You can use this guide as an inspiration. Feel free to add remove wathever you like and keep in mind that every guide is in essence an opinionated."Swift is simular to Objective-C and in many (fascinatingly new) ways it is also very different.
Use this guide as an inspiration to review Swift code"When you review code of others you do not have to review everything. But these are things to look out for:

1. Delegates implemented in extensions?

class Foo {
}
extension Foo : Delegate {
	func delegateMethod (){
	}
}

2. Are all methods that should be private, private?
3. Is the use of optionals minimal?

The use of optionals can be very powerful. However if you make a class the more variables are optional the more you have to scatter around ?! if let ... in your code. So as a pragmatic rule:

  • 1. Make any variable let and required (Type!) in a class
  • 2. If after a while it has to be var do that
  • 3. If it can be nil, make it optional
  • 4. In unit tests you use ! forced unwrap more because if it crashes on the unwrap this also means the test fails. As an example see
    testForcedUnwrap
class Foo() {
    func getOptional() -> Type? {
      return Type?()
    }
  }
  func testForcedUnwrap(){
    let foo = Foo()
    let optional = foo.getOptional()! //a crash here means the test failed

    XCTAssertNotNil(optional)
  }

4. ViewControllers typically get their data right after initialization. It is ok to make the type of required data (Type)!. This avoids the annoying bugs when you ask yourself: "Why the hell is this nil!"

var foo Type!

This includes @IBOutlet's

 @IBOutlet weak var view: UIView!
This means that when you do not provide the data that the UIViewController needs, for example in *prepareForSegue* your app will crash. Also look at the next point. This is the preferred behavior!

5. Is the use of ! forced unwrap rational and not *crashable*.

You are allowed to use ! in places where you want the app to crash because there is no alternative way to continue that makes sense to the user of the app. As an example see DetailViewController

class DetailViewController: UIViewController {
    var detailData: YourDataType!
    @IBOutlet var label: UILabel!

    override func viewDidLoad() {
      super.viewDidLoad()
      label.text = detailData.getLabelText() // Whithout your detailData the VC has no you. So crash.
    }
  }

6. Use of Any or AnyObject should be minimal. Use specific types or protocols. Or even better use generics:

class Foo {
  func generic  (bar: Type) -> Type {
    return bar
  }
}
Foo().generic("Return string")

7. Swift files should only be included in the app target, never in the test target

8. Are only methods public that need to be tested? Or when using XCode 7 use @testableWhen you have to use self in Swift it probably means that you could have a memory leak!. Always think of memory management!

https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/AutomaticReferenceCounting.html

An example:

class Foo {
	let bar = Bar()
	bar.closeAsynchronous{ [unowned self]
		self.barIsClosed()
	}

	func barIsClosed(){
		println("Bar")
	}
}

The code above has a retain cycle in it. The trailing closure closeAsynchronous{...} is called after some time without blocking the main queue. The [unowned self] is needed to brake the cycle.
If bar is not a variable of Foo then we do not need [unowned self]As a wrap up we want to share some of the advantages and disadvantages we found when comparing Swift to Objective-C.

1. Only use "self." when necessary (i.e., in closures)

2. Use "let" instead of "var" whenever possible (tip: try "let" and let the compiler check)

3. The fact that you can no longer send messages to nil makes it difficult at first but after a while you notice that this is actually a bless

4. Compile times are much longer but then again the compiler can catch more bugs.

5. In the beginning it feels like you are trowing arround ! and ? all over the place. But hang in there, after a while it becomes logical.

6. Swift is more familiar to other developers. There is much less resistance to learning Swift as there is to learning Objective-C

7. Swift will be open source

8. Objective-C has method swizzling. Swift has no run time so no swizzling. Actually this is a bless because you have to think of more readable solutions. Sometimes there is no solution but then you can fall back to Objective-C, but try to avoid that.

9. Mocking and stubbing in unit tests is significantly harder but when you do it by using Mock subclassing it is easier to understand for people unfamiliar with swizzling.

10. Swift is fun and Objective-C is fun. But besides the compile time it is worth the effort.