Swift Rewrite Challenge

UPDATE: I reworked this code into a better, more complete version here.

A few days ago, Erica Sadun posted a Swift function snippet and then rewrote it in her own style, discussing the changes. I decided to give it a shot, and (without looking at her version!) refactored the original function to suit my taste. You can find the original snippet here, my version here, and Erica's version on her blog. I also encourage you to try it yourself before looking at her or my version!

Swift 3.0

I have mostly been writing Swift 3 in the past couple months, so I used DEVELOPMENT-SNAPSHOT-2016-05-31-a for my code, and set up a basic Single-View iOS Application to test.

There were a few breaking changes using Swift 3:

  • CGSizeEqualToSize ( CGSize size1, CGSize size2 ) has been changed to a CGSize instance method: func equalTo(_ size2: CGSize) -> Bool
  • CGPointZero was changed to CGPoint.zero (although I didn't end up using it in my revision)
  • Similarly, CGRectZero is now CGRect.zero (also unused)
  • UIImage's drawInRect changed to public func draw(in rect: CGRect)

These are pretty straightforward, so we can just modify them and keep going.

Diving In

My code has a few more comments than the original, but not as many as Erica's. In general, I feel that comments should be a primer for the following code, or serve as an explanation of seemingly strange decisions. Since I was writing this in my own style that I would use for production code, I didn't write comments that I felt should be obvious to a programmer with any experience. For example, both the original and Erica's code contain comments in the first size check:

Original:

// Sanity check; make sure the image isn't already sized.
if CGSizeEqualToSize(image.size, size) {
  return image
}

Erica:

// Return original when cropping is not needed
guard !CGSizeEqualToSize(image.size, size) else { return image }

I felt that this was very clear, and did not warrant a comment.

Also, where the original code linked to StackOverflow to explain the usage of UIGraphicsBeginImageContextWithOptions, I preferred to provide a brief explanation of the reasoning. I think my simple one-line comment adequately explains it, and does not require the reader to open a link and leave the code.

I considered using a guard for the initial check, since I agree that conceptually, it fits better. However, I disliked the double negative in guard !image.size.equalTo(newSize) else {}. It requires too much mental power where almost none should be needed. I kept a simple if-statement, which reads much more naturally to me. Maybe I'm just not in a Swifty enough mindset just yet.

One place that Erica and I agreed upon is that neither of us used any variables. All of the values used are fixed, and I thought that the original snippet's use of variables as a kind of extended conditional initialization was a bit messy. It opened up the values to mutation, when they really should not be.

I also preferred a more declarative style, removing all conditionals. I find

let scaleFactor = max(widthFactor, heightFactor)

to be much clearer than the original with the conditional value change, and even Erica's ternary operator initialization. Erica and I both decided to initialize the drawing origin without conditionals, and I think that her multi-line approach is easier to read than mine on a single line.

Her use of tuples for the width/height pair values is really cool. This never even crossed my mind, and I really like how it lets you have two named values, while mentally associating them. Having both values calculated on a single line doesn't bother me here, since the math is simple and you would see that it is duplicated anyway. However, you could definitely break it up into multiple lines if it were longer.

Swift Style

I don't think my implementation is very Swifty – I am mostly writing Objective-C and Java during the day, and haven't been immersed in Swift constructs enough to naturally where to apply certain techniques. In addition to the already-discussed guard and tuple usages, Romain Menke posted a version using the defer statement to put the UIGraphics image context functions next to each other:

UIGraphicsBeginImageContextWithOptions(newSize, false, 0.0)
defer { UIGraphicsEndImageContext() }

This is a really interesting idea, especially when combined with Erica's encapsulation of the image drawing in a do clause. Since defer will run the code at the end of the current scope, you could use it like this:

let scaledImage: UIImage
do {
   UIGraphicsBeginImageContextWithOptions(size, false, 0.0)
   defer { UIGraphicsEndImageContext() }
   // Fill background
   UIColor.blackColor().setFill(); UIRectFill(CGRect(origin: .zero, size: size))

   // Draw scaled image
   image.drawInRect(drawingRect)

    // Fetch image
    scaledImage = UIGraphicsGetImageFromCurrentImageContext()!
}
return scaledImage

This is kind of a nice way to position the wrapping image context functions, since the do clause already demarcates the drawing code.

Evolution

Some people decided to add more functionality, or move the code into a UIImage extension. I took the refactoring more literally and kept it mostly the same, with practical changes. If you have style edits, or especially more Swift-specific ideas, show them to me on Twitter (@rosslebeau) or in the comments here. Even if they're not pragmatic, I'd love to see some experimental takes on technique.