patternswiftMinor
Sierpinski Triangle
Viewed 0 times
trianglesierpinskistackoverflow
Problem
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
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=1 {
triangleTipLocat
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=1 {
triangleTipLocat
Solution
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.0Next 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.
@IBDesignableI 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.-
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.-
Don't use
get to start your method names. We just don't do it in Swift or Objective-C.-
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.-
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) -> IntSo, 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) ?? 0Code Snippets
func midPoint(point1: CGFloat, point2: CGFloat) -> CGFloat {
return (point1 + point2) / 2
}func average(values: CGFloat...) -> CGFloat {
return values.reduce(0, combine: +) / CGFloat(values.count)
}let mid = average(left.x, right.x)let avg = average(10.0, 20.0, 30.0, 40.0, 50.0)
// avg is 30.0class 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
}Context
StackExchange Code Review Q#96279, answer score: 4
Revisions (0)
No revisions yet.