Method that renders all existing drawn lines on the view

I have several tabbed views with a Unity scene in them. What I do between scene is hide the elements in that scene, then render them if the right view is shown.

The method below is the method that renders all the lines previously drawn in my view. The lines can be lines, angles, points, etc..

```
private void RenderLines()
{
try
{
// Draw all lines & points
if (lines != null)
{
Vector3 sp;
Vector3 sp1;
Vector3 sp2;

createLineMaterial();
lineMaterial.SetPass(0);

// Draw lines
GL.PushMatrix();
GL.LoadPixelMatrix();
GL.Begin(GL.QUADS);
for (int i = 0; i < lines.Count; i++)
{
// solid line
if (lines[i].elementType == ElementType.Line && !lines[i].lineParams.lineName.Contains("IntersectingLine"))
{
sp1 = camera.WorldToScreenPoint(lines[i].point1);
sp2 = camera.WorldToScreenPoint(lines[i].point2);
if (lines[i].lineParams.endPoint1Type == EndPointType.HollowCircle)
{
float endpointRadius = lines[i].lineParams.pointThickness;
Vector3 diff = (sp2 - sp1).normalized * endpointRadius;
sp1 += diff;
}
else if (lines[i].lineParams.endPoint1Type == EndPointType.DoubleCircle || lines[i].lineParams.endPoint1Type == EndPointType.DoubleHollowCircle)
{
float endpointRadius = lines[i].lineParams.pointThickness * doubleCircleRadiusRatio;
Vector3 diff = (sp2 - sp1).normalized * endpointRadius;
sp1 += diff;
}
if (lines[i

DO NOT CATCH GENERAL EXCEPTION TYPES

Seriously, this is very important. You're logging it, which is nice, but do not just catch them.

Imagine this scenario. Somebody calls RenderLines but some of the state is invalid. Nothing appears on screen, but their code keeps on running. They're calling RenderLinesin their loop method, and the game is getting noticeably slower. They check the logs and find millions of generic exception messages all saying something like NullReferenceException.

This is horrible and a pain to debug, especially if you end up calling RenderLines in multiple places.

When you catch your exceptions for logging purposes, rethrow that exception. An exception is like a fish. If you catch a fish but have no intention of eating it, let it go again. Don't throw it in a basket and then dump it out the car on the way back. Only keep the fish you intend to deal with.

catch(Exception ex)
    {
        Log.Item(ex);
        throw;
    }

There are exceptions to this rule, but they are few, and certainly do not apply here.

Foreach

You're referencing lines[i] a lot in your loops, so much so that I think it would be cleaner for you to use a foreach loop instead.

foreach (var line in lines)
{
    // solid line
    if (line.elementType == ElementType.Line && !line.lineParams.lineName.Contains("IntersectingLine"))
        {
        etc...

Break up that method!

You're right, the method is huge, but there's no reason it has to be. You've conveniently delimited the major functionality segments in your method with comments, e.g. // solid line and // dashed line. Instead of having those comments there, why not extract those chunks into private methods and just call them?

StackExchange Code Review Q#91856, answer score: 5