3
\$\begingroup\$

I am trying to create an area graph from the upper and lower limit values.

First I created two line graph from those points as:

enter image description here

Sample data points are:

 var line1dataPoints:[CGPoint] = [CGPoint(x: 10, y: 120),CGPoint(x: 20, y: 130),CGPoint(x: 20, y: 120),CGPoint(x: 70, y: 130),CGPoint(x: 70, y: 120),CGPoint(x: 80, y: 130),CGPoint(x: 90, y: 120),CGPoint(x: 100, y: 110),CGPoint(x: 120, y: 120),CGPoint(x: 120, y: 130)]
 var line2dataPoints:[CGPoint] =  [CGPoint(x: 10, y: 170),CGPoint(x: 20, y: 180),CGPoint(x: 20, y: 170),CGPoint(x: 70, y: 180),CGPoint(x: 70, y: 170),CGPoint(x: 80, y: 180),CGPoint(x: 90, y: 170),CGPoint(x: 100, y: 180),CGPoint(x: 120, y: 160),CGPoint(x: 120, y: 170)]

Method to create those line graphs are:

func addFirstLine(){

        let graphPath = UIBezierPath()
        graphPath.moveToPoint(line1dataPoints[0])

        for i in 1..<line1dataPoints.count {

            let nextPoint = line1dataPoints[i]
            graphPath.addLineToPoint(nextPoint)

        }
        let lineLayer = CAShapeLayer()
        lineLayer.lineWidth = 1.0
        lineLayer.path = graphPath.CGPath
        lineLayer.strokeColor = UIColor.blueColor().CGColor
        lineLayer.fillColor = UIColor.clearColor().CGColor
        self.view.layer.addSublayer(lineLayer)

    }

    func addSecondLine(){

        let graphPath = UIBezierPath()
        graphPath.moveToPoint(line2dataPoints[0])

        for i in 1..<line2dataPoints.count {

            let nextPoint = line2dataPoints[i]
            graphPath.addLineToPoint(nextPoint)

        }
        let lineLayer = CAShapeLayer()
        lineLayer.lineWidth = 1.0
        lineLayer.path = graphPath.CGPath
        lineLayer.strokeColor = UIColor.redColor().CGColor
        lineLayer.fillColor = UIColor.clearColor().CGColor
        self.view.layer.addSublayer(lineLayer)
    }

I created an area graph with the method as:

func createAreaGraph(){

    let rev = Array(line2dataPoints.reverse())
    line1dataPoints.appendContentsOf(rev)

    let graphPath = UIBezierPath()
    graphPath.moveToPoint(line1dataPoints[0])

    for i in 1..<line1dataPoints.count {

        let nextPoint = line1dataPoints[i]
        graphPath.addLineToPoint(nextPoint)

    }
    let lineLayer = CAShapeLayer()
    lineLayer.path = graphPath.CGPath
    lineLayer.fillColor = UIColor.grayColor().CGColor
    self.view.layer.addSublayer(lineLayer)
}

The results:

enter image description here

However, I am wondering if this is the right way to create an area graph or not.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

To me, this seems like a perfectly good way to create the area graph! I think you've done something that's simple, straightforward, and easy to understand. Nice work!

Don't Repeat Yourself

The one thing that I think could be improved is the drawing of the line graphs. You have 2 functions which look identical except one is using line1dataPoints and the other is using line2dataPoints as its input. If you ever decide that something needs to change in the drawing or you need to add or remove anything, you now have to do it in 2 places. Why not just have a single function and pass in the array you want to use? Something like this:

func drawLineGraph(let dataPoints: [CGPoint]) {

    let graphPath = UIBezierPath()
    graphPath.moveToPoint(dataPoints[0])

    for i in 1..<dataPoints.count {

        let nextPoint = dataPoints[i]
        graphPath.addLineToPoint(nextPoint)

    }

    let lineLayer = CAShapeLayer()
    lineLayer.lineWidth = 1.0
    lineLayer.path = graphPath.CGPath
    lineLayer.strokeColor = UIColor.redColor().CGColor
    lineLayer.fillColor = UIColor.clearColor().CGColor
    self.view.layer.addSublayer(lineLayer)
}

Naming

You probably noticed that I renamed the function above to something more descriptive. Your original name was kind of vague. addFirstLine() leaves me wondering what you're adding it to?

But you are actually adding a layer to something at the end of the function. That's kind of surprising! Your original name implied a line was being added to... something. But really what's being added is a layer! And this is a side effect of the function!

Avoid Side Effects

If you want to add a layer to another layer, you should make that explicit in the function's name and definition. You should be either returning the layer that needs to be added, or passing in the layer to add the new layer to. As it is, this function creates a drawing path, then creates a layer to draw it into, then adds it to another layer. But the function definition looks like it just examines some data points. A better function definition might look something like this:

func addLineLayer(let dataPoints: [CGPoint], to layer: CALayer)

Now when you call it, you'll write:

addLineLayer(line1DataPoints, to:self.view.layer)

and it's much more clear to a reader what's going on. (Personally, I'd rename line1DataPoints to something more descriptive, too. Data points from what? Do they represent some maximum or minimum values?

\$\endgroup\$
1
  • \$\begingroup\$ i was thinking about the logic implementation to create area graph..I will keep in mind about the refactoring and naming conventions. Thanks.. \$\endgroup\$ Oct 5, 2016 at 4:05

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.