Sierpinski Triangle

Question

I am a Java programmer who recently started learning some Swift. I made an app that displays the Sierpinski triangle and adds triangles if the user pans up, and zooms in and out using the pinch gesture. I am wondering what I can do to make my code more Swift-y and how I can improve the zooming.

SierpinskiViewController

import UIKit

class SierpinskiViewController: UIViewController, SierpinskiViewDataSource {

private struct Constants {
    static let defaultNumTriangles = 0
    static let numDivideConstant = 48
}

@IBOutlet weak var sierpinskiView: SierpinskiView! {
    didSet {
        sierpinskiView.dataSource = self
        sierpinskiView.addGestureRecognizer(UIPinchGestureRecognizer(target:     sierpinskiView, action: "scale:"))
    }
}

@IBAction func setNumTriangles(gesture: UIPanGestureRecognizer) {
    switch gesture.state {
    case .Ended: fallthrough
    case .Changed:
        let translation = gesture.translationInView(sierpinskiView)
        let numTrianglesChange = -Int(translation.y) / Constants.numDivideConstant
        if numTrianglesChange != 0 {
            numTriangles += numTrianglesChange
            gesture.setTranslation(CGPointZero, inView: sierpinskiView)
        }
    default: break
    }
}

private var numTriangles = Constants.defaultNumTriangles {
    didSet {
        numTriangles = min(max(numTriangles, 0), 10)
        updateUI()
    }
}

private func updateUI() {
    sierpinskiView.setNeedsDisplay()
}

func getNumTriangles(sender: SierpinskiView) -> Int? {
    return numTriangles
}
}

SierpinskiView

import UIKit

protocol SierpinskiViewDataSource: class {
func getNumTriangles(sender: SierpinskiView) -> Int?
}

@IBDesignable
class SierpinskiView: UIView {

var triangleCenter : CGPoint {
    return convertPoint(center, fromView: superview)
}

var scale : CGFloat = 0.95 {
    didSet { setNeedsDisplay() }
}

var triangleTipLocation : CGFloat = 100 {
    didSet {
        if triangleTipLocation<0 {
            triangleTipLocation = 0
        }
        setNeedsDisplay()
    }
}

override func drawRect(rect: CGRect) {
    draw()
}

weak var dataSource : SierpinskiViewDataSource?

private func draw() {

    let bigTriangleRightX : CGFloat = bounds.size.width * scale
    let bigTriangleLeftX : CGFloat = bounds.size.width - bigTriangleRightX
    let triangleHeight : CGFloat = triangleTipLocation + sqrt(((midPoint(bigTriangleLeftX, point2: bigTriangleRightX) - bigTriangleLeftX)*((midPoint(bigTriangleLeftX, point2: bigTriangleRightX) - bigTriangleLeftX)) * 3))

    let triangleTop = CGPoint(x: midPoint(bigTriangleLeftX, point2: bigTriangleRightX), y: triangleTipLocation)
    let rightTriangleCorner = CGPoint(x: bigTriangleRightX, y: triangleHeight)
    let leftTriangleCorner = CGPoint(x: bigTriangleLeftX, y: triangleHeight)

    drawTriangle(leftTriangleCorner, right: rightTriangleCorner, mid: triangleTop, color: .Black)
    drawWhiteTriangle(leftTriangleCorner, rightPoint: rightTriangleCorner, topPoint: triangleTop)
}

private func drawWhiteTriangle(leftPoint: CGPoint, rightPoint: CGPoint, topPoint: CGPoint) {

    let middle = midPoint(leftPoint.x, point2: rightPoint.x)

    let leftCorner = CGPoint(x: midPoint(middle, point2: leftPoint.x), y: midPoint(leftPoint.y, point2: topPoint.y))
    let rightCorner = CGPoint(x: midPoint(middle, point2: rightPoint.x), y: midPoint(rightPoint.y, point2: topPoint.y))
    let midCorner = CGPoint(x: middle, y: leftPoint.y)
    let numberTriangles = dataSource?.getNumTriangles(self) ?? 0
    drawTriangle(leftCorner, right: rightCorner, mid: midCorner, color: .White)
    drawRecursiveTriangles(leftCorner, rightPoint: rightCorner, middlePoint: midCorner, numTriangles: numberTriangles)
}

private func drawRecursiveTriangles(leftPoint: CGPoint, rightPoint: CGPoint, middlePoint: CGPoint, numTriangles: Int) {
    if numTriangles==0 {
        return
    }

    let leftLeftCorner = CGPoint(x: leftPoint.x - (midPoint(leftPoint.x, point2: middlePoint.x)-leftPoint.x), y: midPoint(middlePoint.y, point2: leftPoint.y))
    let leftRightCorner = CGPoint(x: midPoint(middlePoint.x, point2: leftPoint.x), y: midPoint(middlePoint.y, point2: leftPoint.y))
    let leftMidCorner = CGPoint(x: midPoint(leftLeftCorner.x, point2: leftRightCorner.x), y: middlePoint.y)
    drawTriangle(leftLeftCorner, right: leftRightCorner, mid: leftMidCorner, color: .White)
    drawRecursiveTriangles(leftLeftCorner, rightPoint: leftRightCorner, middlePoint: leftMidCorner, numTriangles: numTriangles-1)

    let rightLeftCorner = CGPoint(x: midPoint(rightPoint.x, point2: middlePoint.x), y: midPoint(leftPoint.y, point2: middlePoint.y))
    let rightRightCorner = CGPoint(x: rightPoint.x + (rightPoint.x-midPoint(middlePoint.x, point2: rightPoint.x)), y: midPoint(leftPoint.y, point2: middlePoint.y))
    let rightMidCorner = CGPoint(x: midPoint(rightLeftCorner.x, point2:rightRightCorner.x), y: middlePoint.y)
    drawTriangle(rightLeftCorner, right: rightRightCorner, mid: rightMidCorner, color: .White)
    drawRecursiveTriangles(rightLeftCorner, rightPoint: rightRightCorner, middlePoint: rightMidCorner, numTriangles: numTriangles-1)

    let midLeftCorner = CGPoint(x: midPoint(middlePoint.x, point2: leftPoint.x), y: leftPoint.y - (middlePoint.y-midPoint(leftPoint.y, point2: middlePoint.y)))
    let midRightCorner = CGPoint(x: midPoint(middlePoint.x, point2: rightPoint.x), y: leftPoint.y - (middlePoint.y-midPoint(leftPoint.y, point2: middlePoint.y)))
    let midMidCorner = CGPoint(x: midPoint(leftPoint.x, point2: rightPoint.x), y: leftPoint.y)
    drawTriangle(midLeftCorner, right: midRightCorner, mid: midMidCorner, color: .White)
    drawRecursiveTriangles(midLeftCorner, rightPoint: midRightCorner, middlePoint: midMidCorner, numTriangles: numTriangles-1)
}

private enum Color {case Black, White}

private func drawTriangle(left: CGPoint, right: CGPoint, mid: CGPoint, color: Color) {
    switch color {
    case .White : UIColor.whiteColor().set()
    case .Black : UIColor.blackColor().set()
    }
    let path = UIBezierPath()
    path.moveToPoint(left)
    path.addLineToPoint(right)
    path.addLineToPoint(mid)
    path.fill()
    path.stroke()
}

func scale(gesture: UIPinchGestureRecognizer) {
    if gesture.state == .Changed {
        scale *= gesture.scale
        if scale>=1 {
            triangleTipLocation -= 0.5 * gesture.scale
        } else {
            triangleTipLocation += gesture.scale
        }
        gesture.scale = 1
    }
}

func midPoint(point1: CGFloat, point2: CGFloat) -> CGFloat {
    return (point1 + point2) / 2
}

}

Answer 1

func midPoint(point1: CGFloat, point2: CGFloat) -> CGFloat {
    return (point1 + point2) / 2
}

This is the first thing I see that bothers me.

First of all, this is a "helper function" and has no business belonging in this class. This should be either a global function, or better yet an extension method. But worse, being called midPoint makes it sound like it should take two points and return a point. But it doesn't.

I realize that "midpoint" is a pretty common term when talking about finding the halfway point between any two anythings, but in the context of drawing code, it's confusing.

And really, all this is is a special case of the average function.

So, take the method out of this class, make it a global function that takes an array of CGFloats:

func average(values: CGFloat...) -> CGFloat {
    return values.reduce(0, combine: +) / CGFloat(values.count)
}

Example usage:

let mid = average(left.x, right.x)

So this works perfectly for your special case of two values, but can take any number of values:

let avg = average(10.0, 20.0, 30.0, 40.0, 50.0)
// avg is 30.0

---

Next thing I see that bothers me is the scattering of different types of class members.

You have a handful of properties, then randomly drawRect, then another property, then a handful of methods, and then all of a sudden, you have a nested enum, then still more methods.

And what's worse, there are no comments and no // MARK: - Section Name Here comments to give this any sense of organization which it desperately needs.

Typically, in Swift, we prefer to see classes organized in this manner:

class ClassName : ParentClass, ConformingProtocols, InAlphabeticalOrder {
    // Nested types
    // MARK: - Properties
    // IBOutlet properties
    // IBOutlet collection properties
    // Properties
    // MARK: - Initializers
    // Deinit
    // Initializers
    // MARK: - Life Cycle
    // Lifecycle-type methods (of which drawRect would count here)

    // Any other methods.  They should have logical groupings.
    // And these logical groups should be started with a // MARK: - comment
}

And you'll note, everything within the class is indented here.

---

@IBDesignable

I love @IBDesignable. I jumped on that bandwagon pretty early. But... first, why not make some of your properties @IBInspectable as well?

And also... have you tested this in interface builder? Can interface builder handle drawing these triangle recursively? You might want to put Xcode's Interface Builder through some stress-testing if you're going to leave @IBDesignable on there.

---

protocol SierpinskiViewDataSource: class {
func getNumTriangles(sender: SierpinskiView) -> Int?
}

Several comments here.

First, it's great that you're already comfortable with the idea of datasources and delegates in Swift. This is a pretty important concept through all of iOS/OSX programming and you've pretty much used it correctly here. However... some comments.

1. In Swift, just as in Objective-C, we prefer long, verbose method names. There's no reason here to abbreviate from Number to num. Xcode's code completion is usually quite good and will take care of filling out those long method names. What's important is that the method name is as explicit as possible to any English speaker what the method is doing.

2. Don't use get to start your method names. We just don't do it in Swift or Objective-C.

3. When asking a datasource for information, be sure to pass self. A datasource could be the datasource for numerous instances of the same class, and it can't figure out which value to return unless it knows which instance is asking for it.

4. When in doubt, look to the existing frameworks. The first three comments here are all moot points if we just look to an existing datasource in UIKit and see how it asks for information from its datasource. Consider UITableViewDataSource's numberOfSectionsInTableView::

   optional func numberOfSectionsInTableView(_ tableView: UITableView) -> Int
   

So, with all these points in mind (plus proper indentation), how should our protocol look?

protocol SierpinskiViewDataSource: class {
    func numberOfTrianglesInSierpinskiView(_ sierpinskiView) -> Int
}

Making the return value an optional doesn't make sense. It'd make more sense to make the method itself optional.

@objc protocol SierpinskiViewDataSource: NSObject {
    optional func numberOfTrianglesInSierpinskiView(_ sierpinskiView) -> Int
}

And now calling it ends up looking something more like this:

let triangles = datasource?.numberOfTrianglesInSierpinskiView?(self) ?? 0