HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Setting new lines and adding them to a list to be drawn

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
drawnnewaddingsettingandlistthemlines

Problem

I have a method that basically passes in a known drawn line segment in my view and then draws parallel lines n distances away from the passed in original line. This one in particular draws two parallel lines on either side.

It seems to be a lot of repeat code here, so how can I refactor it to a smaller method and avoid redundancy?

```
public List DrawTwoParallelLines(Line line)
{
List listOfParallelLines = new List();

Line parallelLine1 = new Line();
Line parallelLine2 = new Line();
Line parallelLine3 = new Line();
Line parallelLine4 = new Line();

parallelLine1.point1.x =(float)(line.point1.x + 50 * Math.Cos(90));
parallelLine1.point1.y = (float)(line.point1.y + 0 * Math.Cos(90));
parallelLine1.point2.x = (float)(line.point2.x + 50 * Math.Cos(90));
parallelLine1.point2.y = (float)(line.point2.y + 0 * Math.Cos(90));

parallelLine2.point1.x = (float)(line.point1.x + 100 * Math.Cos(90));
parallelLine2.point1.y = (float)(line.point1.y + 0 * Math.Cos(90));
parallelLine2.point2.x = (float)(line.point2.x + 100 * Math.Cos(90));
parallelLine2.point2.y = (float)(line.point2.y + 0 * Math.Cos(90));

parallelLine3.point1.x = (float)(line.point1.x + -50 * Math.Cos(90));
parallelLine3.point1.y = (float)(line.point1.y + 0 * Math.Cos(90));
parallelLine3.point2.x = (float)(line.point2.x + -50 * Math.Cos(90));
parallelLine3.point2.y = (float)(line.point2.y + 0 * Math.Cos(90));

parallelLine4.point1.x = (float)(line.point1.x + -100 * Math.Cos(90));
parallelLine4.point1.y = (float)(line.point1.y + 0 * Math.Cos(90));
parallelLine4.point2.x = (float)(line.point2.x + -100 * Math.Cos(90));
parallelLine4.point2.y = (float)(line.point2.y + 0 * Math.Cos(90));

listOfParallelLines.Add(parallelLine1);
listOfParallelLines.Add(parallelLine2);
listOfParallelLines.Add(parallelLine3);
listOfParallelLines.Add(parallelLi

Solution

Do you know about radians?

Math.Cos (and the other trig functions in .Net) are based on radians not degrees.

Math.Cos(90) radians is roughly Math.Cos(5157) in degrees. Which is a bit odd...

There are radians in a circle so 1 degree is π/180. As long as you use the same value the lines will be parallel anyway but I thought it was worth pointing out.

Why do you need the Cos at all?

You have a line from (x1, y1) to (x2, y2) with length L = Math.Sqrt((x1-x2)^2 +(y1-y2)^2).

You know that you need to offset the line by an amount (offset) orthogonal to the current line.

private Line CreateParallelLine(Line target, int offset)
{
    var parallelLine = new Line();
    var xDifference = target.point1.x - target.point2.x;
    var yDifference = target.point1.y - target.point2.y;
    var length = Math.Sqrt(Math.Pow(xDifference, 2) + Math.Pow(yDifference, 2));

    parallelLine.point1.x = target.point1.x - offset * yDifference/length;
    parallelLine.point2.x = target.point2.x - offset * yDifference/length;
    parallelLine.point1.y = target.point1.y + offset * xDifference/length;
    parallelLine.point2.y = target.point2.y + offset * xDifference/length;

    return parallelLine;
}


Edit:

I realised that I've only given you an opportunity to create a single parallel line but I think it's straightforward to call the method more times with different offsets!

You didn't show your Line class/struct but it's worth noting that point1 should be at least PascalCase (Point1) but preferably renamed to something more like: Start and point2 renamed to be End or similar.

As x and y are also public, you should PascalCase them too: X and Y.

Code Snippets

private Line CreateParallelLine(Line target, int offset)
{
    var parallelLine = new Line();
    var xDifference = target.point1.x - target.point2.x;
    var yDifference = target.point1.y - target.point2.y;
    var length = Math.Sqrt(Math.Pow(xDifference, 2) + Math.Pow(yDifference, 2));

    parallelLine.point1.x = target.point1.x - offset * yDifference/length;
    parallelLine.point2.x = target.point2.x - offset * yDifference/length;
    parallelLine.point1.y = target.point1.y + offset * xDifference/length;
    parallelLine.point2.y = target.point2.y + offset * xDifference/length;

    return parallelLine;
}

Context

StackExchange Code Review Q#91342, answer score: 6

Revisions (0)

No revisions yet.