patternswiftMinor
Finger painting code
Viewed 0 times
codefingerpainting
Problem
I have a very simple view that handles touch events and draws accordingly. It's nothing significant, but it does use a bit more CPU than I would like (35%). Again, it is the bare minimum (<90 lines), but I can't really think of a simpler way to draw using user input. What would you change?
```
import UIKit
class DrawView: UIView {
var mainImageView = UIImageView()
var tempImageView = UIImageView()
var r = CGFloat(0)
var g = CGFloat(0)
var b = CGFloat(0)
var a = CGFloat(1)
var lineWidth = CGFloat(5)
var dot = true
var lastPoint = CGPoint.zeroPoint
required init(coder aDecoder: NSCoder) {
super.init(coder: aDecoder)
self.backgroundColor = UIColor.clearColor()
self.multipleTouchEnabled = false
mainImageView.frame = self.frame
tempImageView.frame = self.frame
self.addSubview(mainImageView)
self.addSubview(tempImageView)
}
override func touchesBegan(touches: Set, withEvent event: UIEvent) {
dot = true
if let touch = touches.first as? UITouch {
lastPoint = touch.locationInView(self)
}
}
func drawLineFrom(fromPoint: CGPoint, toPoint: CGPoint) {
UIGraphicsBeginImageContextWithOptions(self.frame.size, false, 0)
let context = UIGraphicsGetCurrentContext()
tempImageView.image?.drawInRect(self.frame)
CGContextMoveToPoint(context, fromPoint.x, fromPoint.y)
CGContextAddLineToPoint(context, toPoint.x, toPoint.y)
CGContextSetLineCap(context, kCGLineCapRound)
CGContextSetLineWidth(context, lineWidth)
CGContextSetRGBStrokeColor(context, r, g, b, a)
CGContextSetBlendMode(context, kCGBlendModeNormal)
CGContextStrokePath(context)
tempImageView.image = UIGraphicsGetImageFromCurrentImageContext()
UIGraphicsEndImageContext()
}
override func touchesMoved(touches: Set, withEvent event: UIEvent) {
dot = false
if le
```
import UIKit
class DrawView: UIView {
var mainImageView = UIImageView()
var tempImageView = UIImageView()
var r = CGFloat(0)
var g = CGFloat(0)
var b = CGFloat(0)
var a = CGFloat(1)
var lineWidth = CGFloat(5)
var dot = true
var lastPoint = CGPoint.zeroPoint
required init(coder aDecoder: NSCoder) {
super.init(coder: aDecoder)
self.backgroundColor = UIColor.clearColor()
self.multipleTouchEnabled = false
mainImageView.frame = self.frame
tempImageView.frame = self.frame
self.addSubview(mainImageView)
self.addSubview(tempImageView)
}
override func touchesBegan(touches: Set, withEvent event: UIEvent) {
dot = true
if let touch = touches.first as? UITouch {
lastPoint = touch.locationInView(self)
}
}
func drawLineFrom(fromPoint: CGPoint, toPoint: CGPoint) {
UIGraphicsBeginImageContextWithOptions(self.frame.size, false, 0)
let context = UIGraphicsGetCurrentContext()
tempImageView.image?.drawInRect(self.frame)
CGContextMoveToPoint(context, fromPoint.x, fromPoint.y)
CGContextAddLineToPoint(context, toPoint.x, toPoint.y)
CGContextSetLineCap(context, kCGLineCapRound)
CGContextSetLineWidth(context, lineWidth)
CGContextSetRGBStrokeColor(context, r, g, b, a)
CGContextSetBlendMode(context, kCGBlendModeNormal)
CGContextStrokePath(context)
tempImageView.image = UIGraphicsGetImageFromCurrentImageContext()
UIGraphicsEndImageContext()
}
override func touchesMoved(touches: Set, withEvent event: UIEvent) {
dot = false
if le
Solution
The first thing that strikes me as odd about your code is having the four single-character variables to represent color components. I would rather see a single
Our code has no sense of organization. We should put our event-handling methods together, rather than separate them by the big
Our
Which means when it's called by an end user, the call looks like this:
But internally, we refer to the points as the
This bit isn't really going to work. If these two views are subviews of the
For a start, their
But even this isn't enough. What happens when
Ultimately, there's no real reason to use our image view subviews. Using Core Graphics, we can add lines like this directly to any view. This Code Review question shows an example of drawing lines directly on a view. The only reason we need an image view is if we want to display a
Even if we want to be able to generate a
drawColor variable with the UIColor type. Not only does this eliminate the need for the setColor method, but it also more easily lets us pull that color out for numerous reasons (make some other thing the same color as our DrawView's current drawColor, or see if the DrawView's drawColor is the same shade as something else, etc. There are plenty of reasons. Then only internal to the specific necessary methods do we break that color down into its components for the necessary Core Graphics function call.Our code has no sense of organization. We should put our event-handling methods together, rather than separate them by the big
drawLine method.Our
drawLine method isn't very nicely named. First of all, we can inspect the method to know that it expects points, and the "from" outside of the parenthesis looks and feels very odd. I'd prefer the method be declared something more like this:drawLine(from fromPoint: CGPoint, to toPoint: CGPoint)Which means when it's called by an end user, the call looks like this:
drawView.drawLine(from: pointA, to: pointB)But internally, we refer to the points as the
fromPoint and the toPoint.mainImageView.frame = self.frame
tempImageView.frame = self.frameThis bit isn't really going to work. If these two views are subviews of the
DrawView and we intend for them to be in the same spot, we need to better.For a start, their
frame property should be set to self's bounds property. For any UIView, the frame describes its position in its superview, so the X and Y will be how far it is from the top left corner. Meanwhile, its bounds property will always have an X and Y of zero. So if I set my subview's frame to be equal to its parent's bounds property, it will be the same size, but it will be positioned to the parent's top left corner--and I think this is what we're actually trying to achieve right?But even this isn't enough. What happens when
self is resized by something? Now our manually sized subviews are no longer the correct size. We need to programmatically add autolayout constraints so that the four edges of these subviews are pinned to its parent view, so they will always be the same size.Ultimately, there's no real reason to use our image view subviews. Using Core Graphics, we can add lines like this directly to any view. This Code Review question shows an example of drawing lines directly on a view. The only reason we need an image view is if we want to display a
UIImage. And there's no need to do so here.Even if we want to be able to generate a
UIImage from what the user has drawn on our DrawView, there are means for doing that in iOS (that question is Objective-C, but it can be done in Swift too). So there really is no reason to be using the image views here at all.Code Snippets
drawLine(from fromPoint: CGPoint, to toPoint: CGPoint)drawView.drawLine(from: pointA, to: pointB)mainImageView.frame = self.frame
tempImageView.frame = self.frameContext
StackExchange Code Review Q#95721, answer score: 7
Revisions (0)
No revisions yet.