Ytdyklly

I've written a method to render a polygon in C# using the SharpDX libraries and would like a review of my code's maintainability and the documentation around it. The method works fine as is, but I feel that perhaps the documentation could be improved, or that there could be simpler and more maintainable method of executing the algorithm.

/// <summary>

/// Render an n-sided polygon from a list of allowed polygons.

/// </summary>

/// <param name="context2D">The 2D context used to render the polygon.</param>

/// <param name="polygon">The n-sided polygon to render.</param>

/// <param name="center">The central position of the polygon in 2D space</param>

/// <param name="radius">The radius of the polygon.</param>

/// <param name="rotation">The global rotation of the polygon (in degrees).</param>

public void RenderPolygon(DeviceContext context2D, PolygonType polygon, Vector2 center, float radius, float rotation) {

    float numberOfPoints = (float)polygon;

    Vector2? firstPoint = null, previousPoint = null, currentPoint = null;

    for (int n = 0; n < numberOfPoints; n++) {

        float theta = ((360.0f / numberOfPoints) * n + rotation) * (float)Math.PI / 180.0f;

        currentPoint = center + new Vector2((float)Math.Cos(theta), (float)Math.Sin(theta)) * radius;



        if (previousPoint != null && currentPoint != null)

            context2D.DrawLine((Vector2)previousPoint, (Vector2)currentPoint, brush);



        previousPoint = currentPoint;

        if (firstPoint == null)

            firstPoint = currentPoint;

    }



    if (firstPoint != null && currentPoint != null)

        context2D.DrawLine((Vector2)currentPoint, (Vector2)firstPoint, brush);

}

With regards to the PolygonType argument, this is an enum with int values for each type; for example:

public enum PolygonType {

    Trigon = 3,

    Tetragon = 4,

    Pentagon = 5

}

There are a total of 30 polygon types offered stopping at Chiliagon which has 1000 sides.

The things I am most interested in are:

• Ensuring documentation provides as much detail as possible (while remaining concise).


• Ensuring that the algorithm is easy to follow and maintain.

The one thing I don't currently like about the algorithm:

• Casting from Vector2? to Vector2 seems redundant (though required in the current case).

Since Vector2's default value is 0, 0, I'm not sure I should use it and have chosen to use null; but since Vector2 is not a nullable type, I have to cast back to the normal Vector2 prior to using my variables.

The most confusing thing in your code is names of arguments and variables and their types.

DeviceContext context2D

Why not deviceContext?

PolygonType polygon

It's not polygon (which is collection of points as I expect), it's polygon type, so name it polygonType

float numberOfPoints = (float)polygon;

Why are you casting to float? Number of points can be non-integer? Also in my opinion it's not good to give another meanings to enum values except what they mean by their names. Your enum defines polygons types, not numbers of points. I recommend to create dictionary:

private static readonly Dictionary<PolygonType, int> NumberOfPoints =

    new Dictionary<PolygonType, int>

    {

        [PolygonType.Trigon] = 3,

        // ...

    };

and use it:

var numberOfPoints = NumberOfPoints[polygonType];

Why are you using for polygon types such strange names and not Triangle and Square?

If it's public API method, add arguments checking using appropriate exceptions (ArgumentOutOfRangeException, InvalidEnumArgumentException and so on). All possible exceptions should be documented via <exception> tags.