7
\$\begingroup\$

One of the first things I did when I learned Objective-C was to create a little canvas with editable angles. Basically, you tap to create points. Every tap connects to the previous tap. So if you tap in 2 spots, a line will be drawn from point 1 to point 2. Tap again, and a line will be drawn from point 2 to point 3.

You can edit angles when you aren't creating them by simply dragging on the handles of existing angles.

Although the code works, it is incredibly inefficient and poorly written (it was my very first project when I was 15). I'm planning on rewriting it just for fun and was thinking about possible implementations, but am curious as to how others might change my original attempt a few years back.

Segment.h

@interface Segment : NSObject

@property CGPoint firstPoint;
@property CGPoint secondPoint;
@property CGPoint thirdPoint;

@end

SegmentView.m

#import "Segment.h"
#import "SegmentView.h"
#import <QuartzCore/QuartzCore.h>

BOOL editing1 = FALSE;
BOOL editing2 = FALSE;
BOOL editing3 = FALSE;
BOOL erased = FALSE;
int radius = 35;
int handleSize = 20;

@interface SegmentView ()

@property CGPoint first;
@property CGPoint second;
@property CGPoint third;

@property (nonatomic, strong) Segment *segmentBeingEdited;

@property (nonatomic, strong) NSMutableArray *tapArray;

@end

@implementation SegmentView {
    UIImage *incrementalImage;
    CGFloat r;
    CGFloat g;
    CGFloat b;
    CGFloat opacity;
    CGFloat strokeWidth;
}
@synthesize first;
@synthesize second;
@synthesize third;

- (id)initWithCoder:(NSCoder *)aDecoder {
    if (self = [super initWithCoder:aDecoder]) {
        [self setMultipleTouchEnabled:NO];
        [self setBackgroundColor:[UIColor clearColor]];
        opacity = 1.0 / 1.0;
        strokeWidth = 5.0;
        r = 0;
        g = 0;
        b = 0;
        first = CGPointZero;
        second = CGPointZero;
        third = CGPointZero;
    }
    return self;
}

- (void)touchesBegan:(NSSet *)touches withEvent:(UIEvent *)event {
    UITouch *touch = [touches anyObject];

    CGPoint point = [touch locationInView:self];

    for (Segment *currentSegment in self.tapArray) {
        int xDistance = abs(point.x - currentSegment.firstPoint.x);
        int yDistance = abs(point.y - currentSegment.firstPoint.y);
        int firstDistance = sqrt((xDistance * xDistance) + (yDistance * yDistance));

        int xDistance2 = abs(point.x - currentSegment.secondPoint.x);
        int yDistance2 = abs(point.y - currentSegment.secondPoint.y);
        int secondDistance = sqrt((xDistance2 * xDistance2) + (yDistance2 * yDistance2));

        int xDistance3 = abs(point.x - currentSegment.thirdPoint.x);
        int yDistance3 = abs(point.y - currentSegment.thirdPoint.y);
        int thirdDistance = sqrt((xDistance3 * xDistance3) + (yDistance3 * yDistance3));

        if (firstDistance <= radius) {
            //NSLog(@"First point matches");
            editing1 = TRUE;
            editing2 = FALSE;
            editing3 = FALSE;
            first = point;
            second = currentSegment.secondPoint;
            third = currentSegment.thirdPoint;
            self.segmentBeingEdited = currentSegment;
            [self setNeedsDisplay];
            [self drawBitmap];
            erased = FALSE;
            return;
        }
        else if (secondDistance <= radius) {
            //NSLog(@"Second point matches");
            editing2 = TRUE;
            editing1 = FALSE;
            editing3 = FALSE;
            first = currentSegment.firstPoint;
            second = point;
            third = currentSegment.thirdPoint;
            self.segmentBeingEdited = currentSegment;
            [self setNeedsDisplay];
            [self drawBitmap];
            erased = FALSE;
            return;
        }
        else if (thirdDistance <= radius) {
            //NSLog(@"Third point matches");
            editing3 = TRUE;
            editing1 = FALSE;
            editing2 = FALSE;
            first = currentSegment.firstPoint;
            second = currentSegment.secondPoint;
            third = point;
            self.segmentBeingEdited = currentSegment;
            [self setNeedsDisplay];
            [self drawBitmap];
            erased = FALSE;
            return;
        }
        else {
            editing1 = FALSE;
            editing2 = FALSE;
            editing3 = FALSE;
        }
    }

    [self setAlpha:opacity];
    if (CGPointEqualToPoint(first, CGPointZero)) {
        first = [touch locationInView:self];
    }
    else if (!CGPointEqualToPoint(first, CGPointZero) && CGPointEqualToPoint(second, CGPointZero)) {
        second = [touch locationInView:self];
    }
    else if (!CGPointEqualToPoint(first, CGPointZero) && !(CGPointEqualToPoint(second, CGPointZero)) && CGPointEqualToPoint(third, CGPointZero)) {
        third = [touch locationInView:self];
    }
    else {
        //[self drawBitmap];
        first = [touch locationInView:self];
        second = CGPointZero;
        third = CGPointZero;
    }

    [self setNeedsDisplay];
}

- (void)touchesMoved:(NSSet *)touches withEvent:(UIEvent *)event {
    UITouch *touch = [touches anyObject];

    CGPoint point = [touch locationInView:self];

    for (int i = 0; i < self.tapArray.count; i++) {
        if (editing1) {
            editing2 = FALSE;
            editing3 = FALSE;
            first = point;
            second = self.segmentBeingEdited.secondPoint;
            third = self.segmentBeingEdited.thirdPoint;
            //self.segmentBeingEdited = currentSegment;
            if (erased == FALSE) {
                [self drawBitmap];
                //      NSLog(@"YOU NEED TO ERASE.");
            }
            [self setNeedsDisplay];
            return;
        }
        else if (editing2) {
            editing1 = FALSE;
            editing3 = FALSE;
            first = self.segmentBeingEdited.firstPoint;
            second = point;
            third = self.segmentBeingEdited.thirdPoint;
            //self.segmentBeingEdited = currentSegment;
            if (erased == FALSE) {
                [self drawBitmap];
                //    NSLog(@"YOU NEED TO ERASE.");
            }
            [self setNeedsDisplay];
            return;
        }
        else if (editing3) {
            //       NSLog(@"It's editing time, yo");
            editing1 = FALSE;
            editing2 = FALSE;
            first = self.segmentBeingEdited.firstPoint;
            second = self.segmentBeingEdited.secondPoint;
            third = point;
            //self.segmentBeingEdited = currentSegment;
            if (erased == FALSE) {
                [self drawBitmap];
                //             NSLog(@"YOU NEED TO ERASE.");
            }
            [self setNeedsDisplay];
            return;
        }
        else {
            editing1 = FALSE;
            editing2 = FALSE;
            editing3 = FALSE;
        }
    }

    if (!CGPointEqualToPoint(first, CGPointZero) && CGPointEqualToPoint(second, CGPointZero)) {
        first = [touch locationInView:self];
    }
    else if (!CGPointEqualToPoint(first, CGPointZero) && !(CGPointEqualToPoint(second, CGPointZero)) && CGPointEqualToPoint(third, CGPointZero)) {
        second = [touch locationInView:self];
    }
    else if (!CGPointEqualToPoint(first, CGPointZero) && !(CGPointEqualToPoint(second, CGPointZero)) && !(CGPointEqualToPoint(third, CGPointZero))) {
        third = [touch locationInView:self];
    }
    else {
        first = [touch locationInView:self];
        second = CGPointZero;
        third = CGPointZero;
    }

    [self setNeedsDisplay];
}

- (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event {
    NSUInteger index;
    if (editing1 || editing2 || editing3) {
        index = [self.tapArray indexOfObject:self.segmentBeingEdited];
        Segment *datSegment = [self.tapArray objectAtIndex:index];
        datSegment.firstPoint = first;
        datSegment.secondPoint = second;
        datSegment.thirdPoint = third;
    }


    if (!CGPointEqualToPoint(third, CGPointZero) && editing1 == FALSE && editing2 == FALSE && editing3 == FALSE) {
        Segment *segment = [[Segment alloc] init];
        segment.firstPoint = first;
        segment.secondPoint = second;
        segment.thirdPoint = third;

        if (self.tapArray == nil) {
            self.tapArray = [[NSMutableArray alloc] init];
        }

        [self.tapArray addObject:segment];
    }

    editing1 = FALSE;
    editing2 = FALSE;
    editing3 = FALSE;
    erased = FALSE;

    /*
       NSLog(@"TapArray: %lu,",(unsigned long)self.tapArray.count);
       for(Segment *segment in self.tapArray){
       NSLog(@"%@,\n%@,\n%@",NSStringFromCGPoint(segment.firstPoint), NSStringFromCGPoint(segment.secondPoint), NSStringFromCGPoint(segment.thirdPoint));
       NSLog(@"NEW ANGLE");
       }
     */

    [self drawBitmap];
}

- (void)drawRect:(CGRect)rect {
    [incrementalImage drawInRect:rect];
    CGContextRef context = UIGraphicsGetCurrentContext();

    CGContextSetRGBStrokeColor(context, r, g, b, opacity);
    CGContextSetLineWidth(context, strokeWidth);
    CGContextSetLineCap(context, kCGLineCapRound);
    CGContextSetLineJoin(context, kCGLineJoinRound);

    if (!CGPointEqualToPoint(first, CGPointZero)) {
        CGRect rectangle = CGRectMake(first.x - 10, first.y - 10, handleSize, handleSize);
        CGContextAddEllipseInRect(context, rectangle);
        CGContextMoveToPoint(context, first.x, first.y);

        if (!CGPointEqualToPoint(second, CGPointZero)) {
            CGContextAddLineToPoint(context, second.x, second.y);
            CGRect rectangle2 = CGRectMake(second.x - 10, second.y - 10, handleSize, handleSize);
            CGContextAddEllipseInRect(context, rectangle2);
            CGContextMoveToPoint(context, second.x, second.y);
        }

        if (!CGPointEqualToPoint(third, CGPointZero)) {
            CGContextAddLineToPoint(context, third.x, third.y);
            CGRect rectangle3 = CGRectMake(third.x - 10, third.y - 10, handleSize, handleSize);
            CGContextAddEllipseInRect(context, rectangle3);

            CGFloat xDistA = (second.x - third.x);
            CGFloat yDistA = (second.y - third.y);
            CGFloat a = sqrt((xDistA * xDistA) + (yDistA * yDistA));

            CGFloat xDistB = (first.x - third.x);
            CGFloat yDistB = (first.y - third.y);
            CGFloat b2 = sqrt((xDistB * xDistB) + (yDistB * yDistB));

            CGFloat xDistC = (second.x - first.x);
            CGFloat yDistC = (second.y - first.y);
            CGFloat c = sqrt((xDistC * xDistC) + (yDistC * yDistC));

            CGFloat tempAngle = acos(((a * a) + (c * c) - (b2 * b2)) / ((2 * (a) * (c))));

            CGFloat angle2 = (tempAngle * 180) / M_PI;
            int angle = (int)angle2;

            NSString *daAngle = [NSString stringWithFormat:@"%d°", angle];

            if (first.y < second.y && first.x <= second.x && second.x < third.x && second.y >= third.y) {
                CGPoint daPoint = CGPointMake(second.x + 25, second.y - 50);

                NSDictionary *textAttributes = @{ NSFontAttributeName: [UIFont boldSystemFontOfSize:25.0],
                                                  NSForegroundColorAttributeName:   [UIColor colorWithRed:r green:g blue:b alpha:opacity] };

                [daAngle drawAtPoint:daPoint withAttributes:textAttributes];
            }
            else if (first.x >= second.x && first.y < second.y && second.y >= third.y) {
                CGPoint daPoint = CGPointMake(second.x + 25, second.y + 25);

                NSDictionary *textAttributes = @{ NSFontAttributeName: [UIFont boldSystemFontOfSize:25.0],
                                                  NSForegroundColorAttributeName:   [UIColor colorWithRed:r green:g blue:b alpha:opacity] };

                [daAngle drawAtPoint:daPoint withAttributes:textAttributes];
            }
            else if (first.y < second.y && second.y >= third.y && third.x < second.x) {
                CGPoint daPoint = CGPointMake(second.x + 25, second.y - 25);

                NSDictionary *textAttributes = @{ NSFontAttributeName: [UIFont boldSystemFontOfSize:25.0],
                                                  NSForegroundColorAttributeName:   [UIColor colorWithRed:r green:g blue:b alpha:opacity] };

                [daAngle drawAtPoint:daPoint withAttributes:textAttributes];
            }
            else {
                CGPoint daPoint = CGPointMake(second.x - 50, second.y + 25);

                NSDictionary *textAttributes = @{ NSFontAttributeName: [UIFont boldSystemFontOfSize:25.0],
                                                  NSForegroundColorAttributeName:   [UIColor colorWithRed:r green:g blue:b alpha:opacity] };

                [daAngle drawAtPoint:daPoint withAttributes:textAttributes];
            }
        }

        CGContextStrokePath(context);
    }
}

- (void)drawBitmap {
    UIGraphicsBeginImageContextWithOptions(self.bounds.size, NO, 0.0);

    if (!incrementalImage) { // first draw;
        UIBezierPath *rectpath = [UIBezierPath bezierPathWithRect:self.bounds]; // enclosing bitmap by a rectangle defined by another UIBezierPath object
        [[UIColor clearColor] setFill];
        [rectpath fill]; // fill it
    }
    [incrementalImage drawAtPoint:CGPointZero];

    CGContextRef context = UIGraphicsGetCurrentContext();


    CGContextSetRGBStrokeColor(context, r, g, b, opacity);
    CGContextSetLineWidth(context, strokeWidth);
    CGContextSetLineCap(context, kCGLineCapRound);
    CGContextSetLineJoin(context, kCGLineJoinRound);

    if (editing1 || editing2 || editing3) {
        //    CGContextBeginTransparencyLayer(context, NULL);
        CGContextSetBlendMode(context, kCGBlendModeClear);
        CGContextSetLineWidth(context, strokeWidth + 1);
        //CGContextSetBlendMode(context, kCGBlendModeColor);

        if (!CGPointEqualToPoint(self.segmentBeingEdited.firstPoint, CGPointZero)) {
            CGContextMoveToPoint(context, self.segmentBeingEdited.firstPoint.x, self.segmentBeingEdited.firstPoint.y);

            if (!CGPointEqualToPoint(self.segmentBeingEdited.secondPoint, CGPointZero)) {
                CGContextAddLineToPoint(context, self.segmentBeingEdited.secondPoint.x, self.segmentBeingEdited.secondPoint.y);
            }

            if (!CGPointEqualToPoint(self.segmentBeingEdited.thirdPoint, CGPointZero)) {
                CGContextAddLineToPoint(context, self.segmentBeingEdited.thirdPoint.x, self.segmentBeingEdited.thirdPoint.y);
            }

            CGContextStrokePath(context);
            //      CGContextEndTransparencyLayer(context);
        }

        incrementalImage = UIGraphicsGetImageFromCurrentImageContext();
        UIGraphicsEndImageContext();
        erased = TRUE;
        return;
    }

    CGContextSetBlendMode(context, kCGBlendModeNormal);


    if (!CGPointEqualToPoint(first, CGPointZero)) {
        CGContextMoveToPoint(context, first.x, first.y);

        if (!CGPointEqualToPoint(second, CGPointZero)) {
            CGContextAddLineToPoint(context, second.x, second.y);
        }

        if (!CGPointEqualToPoint(third, CGPointZero)) {
            CGContextAddLineToPoint(context, third.x, third.y);
        }

        CGContextStrokePath(context);
    }

    incrementalImage = UIGraphicsGetImageFromCurrentImageContext();
    UIGraphicsEndImageContext();
}

- (void)erase {
    incrementalImage = nil;
    [self.tapArray removeAllObjects];
    first = CGPointZero;
    second = CGPointZero;
    third = CGPointZero;
    [self setNeedsDisplay];
}

- (void)setColor:(UIColor *)color {
    CGColorRef activeColor = [color CGColor];

    unsigned long components = CGColorGetNumberOfComponents(activeColor);

    if (components == 4) {
        const CGFloat *components = CGColorGetComponents(activeColor);
        r = components[0];
        g = components[1];
        b = components[2];
        // opacity = components[3];
    }
}

- (void)setAlpha:(CGFloat)alpha {
    opacity = alpha;
}

- (void)setStrokeWidth:(CGFloat)width {
    strokeWidth = width;
}

@end

Looking at how long and unmanageable this code is makes me cringe, but also reminds me of how much I've learned since then.

What would you improve?

\$\endgroup\$
5
\$\begingroup\$

You've dumped a lot of code here, and you mentioned you already intend to rewrite it, but you're interested in some feedback before you start that process. So with that in mind, I'm going to just look at the big picture concepts for the starting point of your rewrite.


First of all, your classes should have some sort of 2-3 letter prefix. Objective-C does not have namespaces, so to prevent name collisions between classes, we use these 2-3 letter prefixes (like you see in your code with NS, CG, and UI [and you've surely seen plenty of others]). So, perhaps come up with a name for your library and use an abbreviated form of that library name for the 2-3 letter prefix. Based on your other question, perhaps this code belongs to a library for finger-painting? Perhaps this is libFingerPaint? So prefix everything with FP. And when in doubt, just use your initials (unless your initials are something like NS [I happen to work with an iOS developer whose first/last name initials are NS]).


Second, your Segment class doesn't actually seem particularly useful, I mean, not really. Not in the grand scheme of things. I know you've posted the SegmentView class and that seems to be your primary area of concern here, but it's all sort of built upon the Segment class, right? It does its work based on how the Segment class works.

Well, more useful would perhaps be a StraightLineSegment class (FPStraightLineSegment) (and arguably, this is just a struct, not a class).

struct FPStraightLineSegment {
    CGPoint startPoint;
    CGPoint endPoint;
}

This is far more useful.

What the plain English part of your question describes is that the user taps to create a point, then taps again to create a second point and draw a line between these points. Managing drawing lines like this when we're pairing lines makes things difficult.

Then, perhaps to manage a chain of line segments, we make a class for managing that. There's probably a more geometrically accurate term for a collection of segments, each starting where the previous began, but...

@interface FPLineChain : NSObject

@property (readonly) NSArray *lineSegments;

- (void)addLineSegment:(FPStraightLineSegment)lineSegment;

// and **lots** of other methods

- (void)drawLineSegmentInView:(UIView *)view;

@end

And there are other approaches rather than simply this drawLineSegmentInView: approach.

Think about the CoreGraphics library (which is where CGPoint, CGRect, CGSize, and a whole lot of other structs and types that UIKit uses come from. Have you ever used a CGView? Nope. The CoreGraphics library simply gives you a lot of tools that you're probably using with UIKit objects to draw some stuff on the screen. So we should at least start by making our library very similar, and only actually implement custom view subclasses if there becomes a really good reason to do so.

So, right now, you're either completely on the same page as me, or you're sitting here thinking about this point you mentioned:

You can edit angles when you aren't creating them by simply dragging on the handles of existing angles.

And you're wondering if I've forgotten about this or what, right?

Well, I haven't. I'm just taking a different approach. Your original implementation (especially looking at the Segment class and your plain-English wording of this question) places a very, very heavy emphasis on the angles we're drawing. But why?

There's no good reason to emphasize angles unless we're doing some geometry app where the measure of the angle actually matters.

What is an "angle" besides simply the place where two line segments meet? So when we tap on a point to move it, are we changing the angle? Or are we simply changing two line segments?

Realistically, it's the latter. And barely that. What if we only have one line and we're trying to adjust one of its end points? There's no angle here. What if we have multiple lines, but we're still adjusting an endpoint? We're still not adjusting an angle. Just changing one of the points.

So, this is where we return to our FPLineChain class. We'll want some methods that look like this:

- (void)changeStartPointAtIndex:(NSUInteger)index toPoint:(CGPoint)point;
- (void)changeEndPointAtIndex:(NSUInteger)index toPoint:(CGPoint)point;

Or perhaps we want a method that's simply something like this:

- (void)changePoint:(CGPoint)oldPoint toPoint:(CGPoint)newPoint;

Now, the changeStartPointAtIndex:toPoint: will go to that index in your lineSegments array, change the start point to the new point. And if that index is non-zero, it will also change the end point of the previous index to the new point.

The reverse happens for the changeEndPointAtIndex:toPoint:. We go to that index and change the end point. If that's not the last index of the array, we go to the next index and update that line segment's startPoint.

If we implement the changePoint:toPoint: method, we simply iterate over the entire array, and any time the oldPoint matches either point on any of your lineSegments, you update that point to newPoint.


As for the way you're handling the line color? I can only point you to the comments I left on your other question:

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 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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.