Calling a method to draw parallel lines

Question

This question is a followup to the last one.

I followed RobH's advice and his edge case bug found in my code with this solution:

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 = (float)(target.point1.x - offset * yDifference/length);
    parallelLine.point2.x = (float)(target.point2.x - offset * yDifference/length);
    parallelLine.point1.y = (float)(target.point1.y + offset * xDifference/length);
    parallelLine.point2.y = (float)(target.point2.y + offset * xDifference/length);

    return parallelLine;
}

Now, since his solution only provided a means of creating one parallel line, even though I need to draw several lines, I have this method that draws the parallel lines (it works) but it is long and there is repeat code:

private void DrawParallelLines(object sender, LineDrawingEventArgs e)
    {
        Line referenceLine = CameraObject.GetComponent<DrawLines>().GetLineParams(ViewType + StickLocationEnum.ToString() + linename);
        Line ParallelLine1 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, 50);
        Line ParallelLine2 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, -50);
        Line ParallelLine3 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, 100);
        Line ParallelLine4 = CameraObject.GetComponent<DrawLines>().CreateParallelLine(referenceLine, -100);

        LineParams paramForParallelLine = new LineParams(ViewType + "ParallelLine", Color.green, Color.green, EndPointType.Circle, EndPointType.Circle, lineThickness, endPointThickness);
        Line parallellineWithParams1 = new Line(ParallelLine1.point1, ParallelLine1.point2, paramForParallelLine, ElementType.Line);
        Line parallellineWithParams2 = new Line(ParallelLine2.point1, ParallelLine2.point2, paramForParallelLine, ElementType.Line);
        Line parallellineWithParams3 = new Line(ParallelLine3.point1, ParallelLine3.point2, paramForParallelLine, ElementType.Line);
        Line parallellineWithParams4 = new Line(ParallelLine4.point1, ParallelLine4.point2, paramForParallelLine, ElementType.Line);

        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams1);
        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams2);
        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams3);
        CameraObject.GetComponent<DrawLines>().AddNewLine(parallellineWithParams4);
        CameraObject.GetComponent<DrawLines>().UpdateAllLabels();
    }

What is the best way to refactor this method to remove redundant code?

Answer 1

One option is to use a foreach loop to iterate over the offsets that you want to use.

private void DrawParallelLines(object sender, LineDrawingEventArgs e)
{
    var linesComponent = CameraObject.GetComponent<DrawLines>();

    var referenceLine = linesComponent.GetLineParams(ViewType + StickLocationEnum.ToString() + linename);
    var lineParams = new LineParams(ViewType + "ParallelLine", Color.green, Color.green, EndPointType.Circle, EndPointType.Circle, lineThickness, endPointThickness);

    foreach (var offset in new[] {50, -50, 100, -100})
    {
        var parallelLine = linesComponent.CreateParallelLine(referenceLine, offset);
        var parallelWithParams = new Line(parallelLine.Point1, parallelLine.Point2, lineParams, ElementType.Line);
        linesComponent.AddNewLine(parallelWithParams);
    }
}

EDIT

What I think Mat is getting at, is that this solution is localised to this usage, and may not be suitable in an application that frequently needs to generate parallel lines. If this is the only place in your app that you create parallel lines, then keep using it (YAGNI).

If however you need to create parallel lines a lot, you could make your implementation a little more general. One way of achieving this could be through extension methods. Consider the following

public static class LineExtensions
{
    public static Line CreateParallelLine(this Line target, int offset)
    {
        ... // Same as in question
    }

    public static IEnumerable<Line> CreateParallelLines(this Line line, params int[] offsets)
    {
        return offsets.Select(o => CreateParallelLine(line, o));
    }

    public static IEnumerable<Line> CreateParallelLines(this Line line, int count, int spacing)
    {
        for (var i = 1; i <= count; i++)
            yield return CreateParallelLine(line, i*spacing);
    }
}

Your foreach then becomes

foreach (var parallel in referenceLine.CreateParallelLines(50, -50, 100, -100))
    ...

Or in the case of generating 300 parallel lines

var positives = referenceLine.CreateParallelLines(150, 50);
var negatives = referenceLine.CreateParallelLines(150, -50);

foreach (var parallel in positives.Concat(negatives))
    ...

Answer 2

How would you handle 300 parallel lines?

    Line ParallelLine1 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 50);
    Line ParallelLine2 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -50);
    Line ParallelLine3 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 100);
    Line ParallelLine4 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -100);
    ...
    Line ParallelLine297 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 950);
    Line ParallelLine298 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -950);
    Line ParallelLine299 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, 1000);
    Line ParallelLine300 = CameraObject.GetComponent<DrawLines>()
                                     .CreateParallelLine(referenceLine, -1000);

Something's not right isn't it?

I think you need to put words on the concepts involved here:

• You have a bunch of lines.
• Lines are drawn in pairs.
• Lines are offset from the referenceLine by a fixed interval - one line with a positive offset, the other with a negative offset.

You need to write a method that can create n lines and return an IEnumerable<Line> given:

• A referenceLine.
• An offsetIncrement for the offset.
• The n number of pairs of lines you want to create.
• A LineParams instance.

And then you can just iterate the result and write the AddNewLine() call only once.