Calculating compass direction [closed]

Question

I have a heading, which is an integer and from it I would like to figure out the compass direction (North, North-East,...) and return the appropriate icon. I feel like I'm probably already at the cleanest solution but I would love to be proven wrong.

public GetHeadingImage(string iconName, int heading){
        if (IsNorthHeading(heading))
            return iconName  + "n" + ICON_FILE_EXTENTION;
        if (IsNorthEastHeading(heading))
            return iconName + "ne" + ICON_FILE_EXTENTION;
        if (IsEastHeading(heading))
            return iconName + "e" + ICON_FILE_EXTENTION;
        /*A bunch more lines*/
}
private bool IsNorthHeading(int heading)
{
    return heading < 23 || heading > 337;
}
/* A bunch more extracted test methods*/

Answer 1

A simple solution is a good solution:

public GetHeadingImage(string iconName, int heading){
    var directions = new string[] {
        "n", "ne", "e", "se", "s", "sw", "w", "nw", "n"
    };

    var index = (heading + 23) / 45;
    return iconName + directions[index] + ICON_FILE_EXTENSION;
}

Answer 2

Small suggestion: the GetHeadingImage() has a lot of duplicate code.
Why not something like:

public GetHeadingImage(string iconName, int heading){
    return iconName  + GetHeadingName(heading) + ICON_FILE_EXTENTION;
}

You could then have the heading logic just inside GetHeadingName().

Answer 3

Two things: 1) Extract GetCompasDirection as a separate method which will return an enum 2) Create a collection of angles and corresponding headers to remove a lot of Is...Heading methods:

public enum CompasDirection
{
    North,
    NorthEast,
    // other directions
}

public CompasDirection GetCompasDirection(int heading)
{
    // directions in clock-wise order:
    var directionUpperLimitAngles = new [] {
         Tuple.Create(CompasDirection.North, 22),
         Tuple.Create(CompasDirection.NorthEast, 67),
         Tuple.Create(CompasDirection.East, 112),
         // other directions,
         Tuple.Create(CompasDirection.North, 360), // north again
    };

    return directionUpperLimitAngles.Last(d => d.Item2 <= heading).Item1;
}

public string GetHeadingImage(string imageName, int heading)
{
    var directionToIconSuffixMapping = new Dictionary<CompasDirection, string> {
        { CompasDirection.North, "n"},
        { CompasDirection.NorthEast, "ne"},
        // other directions
    };
    var direction = GetCompasDirection(heading);
    return iconName + directionToIconSuffixMapping[direction] + ICON_FILE_EXTENTION;
}

Some parts here can be simplify (for example you can remove second dictionary and simply name your icon files correspondingly to enum members).

This approach with direction-heading table if I remember correctly I've taken from McConnel's Code Complete

UPDATE: replaced inner private class with Tuples

Answer 4

Another alternative:

public enum Direction
{
    North = 0,
    NorthEast = 1,
    East = 2,
    SouthEast = 3,
    South = 4,
    SouthWest = 5,
    West = 6,
    NorthWest = 7
}

public static class DirectionExtensions
{
    private static readonly Dictionary<Direction, string>
        mapping = new Dictionary<Direction, string>
    {
        { Direction.North, "n" },
        { Direction.NorthEast, "ne" },
        { Direction.East, "e" },
        { Direction.SouthEast, "se" },
        { Direction.South, "s" },
        { Direction.SouthWest, "sw" },
        { Direction.West, "w" },
        { Direction.NorthWest, "nw" }
    };

    public static bool IncludesHeading(this Direction direction, int heading)
    {
        var adjusted = (heading + 22) % 360;
        var adjMin = (int) direction * 45;
        var adjMax = adjMin + 44;
        return (adjusted >= adjMin && adjusted <= adjMax);
    }

    public static string GetSuffix(this Direction direction)
    {
        return mapping[direction];
    }
}

Leaves your method reading like this:

public string GetHeadingImage(string imageName, int heading)
{
     Direction[] directions = ((Direction[]) Enum.GetValues(typeof(Direction)));
     var match = directions.First(d => d.IncludesHeading(heading));
     return imageName + match.GetSuffix() + ICON_FILE_EXTENTION;
}

[Edit: Taking that one step further]

Replace the IncludesHeading extension with

public static IntDirectionExtensions
{
    public static Direction GetDirection(this int heading)
    {
        var adjusted = (heading + 22) % 360;
        var sector = adjusted / 45;
        return (Direction)sector;
    }
}

And now you can simplify your method to

public string GetHeadingImage(string imageName, int heading)
{
    return imageName + heading.GetDirection().GetSuffix() + ICON_FILE_EXTENTION;
}

[Edit 2: Another idea]

Another thing you could do is map to the suffix via reflection, which I think looks nicer but is probably less efficient

public enum Direction
{
    [IconSuffix("n")] North = 0,
    [IconSuffix("ne")] NorthEast = 1,
    [IconSuffix("e")] East = 2,
    [IconSuffix("se")] SouthEast = 3,
    [IconSuffix("s")] South = 4,
    [IconSuffix("sw")] SouthWest = 5,
    [IconSuffix("w")] West = 6,
    [IconSuffix("nw")] NorthWest = 7
}

public class IconSuffixAttribute : Attribute
{
    public string Suffix { get; private set; }
    public IconSuffixAttribute(string suffix)
    {
         Suffix = suffix;
    }
}

Replacing your GetSuffix extension (and now-defunct Dictionary mapping) with

public static string GetSuffix(this Direction direction)
{
    var suffix = from m in typeof(Direction).GetMember(direction.ToString())
                 from a in m.GetCustomAttributes(typeof(IconSuffixAttribute), false)
                 select ((IconSuffixAttribute) a).Suffix;
    return suffix.First();
}

Everything else remains the same.

Answer 5

You could simplify your solution by concatenating the north/south letter with the east/west letter and thus avoid any need for IsNorthEastHeading and such like.

string northsouth = (heading < 23 || heading > 337) ? "n" :
                    (heading > 157 && heading < 203) ? "s" : 
                    "";
string eastwest = ...
return iconName + northsouth + eastwest + ICON_FILE_EXTENTION;

Is it really worth adding all those extra methods or introducing enums? Personally, I prefer this three line method over all of the much larger solutions proposed.

Answer 6

Naming (minor): Something possibly more specific to the domain for sector boundaries ('left'? and 'right'? relative angles) might be instead be called a radial. Each radial in at least [aeronautical] navigation is referred to as the '#named number of radial# radial', such as 'the 025 radial', or commonly just by number, 'the 025' (read as zero two five). Perhaps this would help minimize magic numbers by declaring your boundaries as named radial constants.

To go a step further, since you are dividing the compass into equally sized parts, or partitions, you might create constant/immutable value objects that describe these partitions. 'CardinalDirection' (n e s w) with public getters of left radial and right radial is an revised offhand suggestion. Ordinals are the next set of directional divisions (ne se sw nw).

Hope this helps refine your model for the better.

Answer 7

A formula that gives the 8-way compass directions. Is working for any value of X and Y .For X=Y=0 the result is undefined. I write the formula in the same way ,that i did in excel.

f(X,Y)=MOD(2*(2-SIGN(Y1)-(1+SIGN(X1))*(1-SIGN(Y1^2)))-SIGN(X1*Y1)-0.5*SIGN(Y1*X1^3-X1*Y1^3)

  *(1+SIGN(ABS(ABS(X1)-ABS(Y1))/(ABS(X1)+ABS(Y1))-2^0.5+1-2^-22)),8) 

The formula gives for the East =0 and NE=1,N=2,NW=3,W=4,SW=5,S=6 and SE=7.