Calculating compass direction












12












$begingroup$


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*/









share|improve this question











$endgroup$












  • $begingroup$
    Is it possible at all that the thing, whatever it may be, is not moving at all?
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:30
















12












$begingroup$


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*/









share|improve this question











$endgroup$












  • $begingroup$
    Is it possible at all that the thing, whatever it may be, is not moving at all?
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:30














12












12








12


1



$begingroup$


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*/









share|improve this question











$endgroup$




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*/






c#






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited 6 mins ago









Jamal

30.3k11119227




30.3k11119227










asked Mar 14 '11 at 20:18









stimmsstimms

20828




20828












  • $begingroup$
    Is it possible at all that the thing, whatever it may be, is not moving at all?
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:30


















  • $begingroup$
    Is it possible at all that the thing, whatever it may be, is not moving at all?
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:30
















$begingroup$
Is it possible at all that the thing, whatever it may be, is not moving at all?
$endgroup$
– Leonid
Aug 9 '12 at 21:30




$begingroup$
Is it possible at all that the thing, whatever it may be, is not moving at all?
$endgroup$
– Leonid
Aug 9 '12 at 21:30










7 Answers
7






active

oldest

votes


















24












$begingroup$

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;
}





share|improve this answer











$endgroup$









  • 9




    $begingroup$
    +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
    $endgroup$
    – k3b
    Mar 15 '11 at 9:53








  • 2




    $begingroup$
    The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:58












  • $begingroup$
    @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
    $endgroup$
    – munificent
    Mar 16 '11 at 1:44










  • $begingroup$
    Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
    $endgroup$
    – Danny Beckett
    Jan 5 '14 at 9:24



















7












$begingroup$

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().






share|improve this answer











$endgroup$









  • 1




    $begingroup$
    Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
    $endgroup$
    – Josh Earl
    Mar 16 '11 at 12:33



















3












$begingroup$

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






share|improve this answer











$endgroup$









  • 1




    $begingroup$
    What use is the enum here? Why not simply map numbers to suffixes?
    $endgroup$
    – pdr
    Mar 14 '11 at 21:34






  • 1




    $begingroup$
    @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
    $endgroup$
    – Snowbear
    Mar 14 '11 at 21:37












  • $begingroup$
    @Snowbear, Good point
    $endgroup$
    – pdr
    Mar 15 '11 at 1:44










  • $begingroup$
    +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:05












  • $begingroup$
    @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
    $endgroup$
    – Snowbear
    Mar 15 '11 at 10:11



















2












$begingroup$

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.






share|improve this answer











$endgroup$













  • $begingroup$
    I don't really find an extension method appropriate here.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:29










  • $begingroup$
    @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
    $endgroup$
    – pdr
    Mar 15 '11 at 10:33










  • $begingroup$
    IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:46










  • $begingroup$
    @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
    $endgroup$
    – pdr
    Mar 15 '11 at 10:53










  • $begingroup$
    I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
    $endgroup$
    – pdr
    Mar 15 '11 at 10:54



















2












$begingroup$

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.






share|improve this answer









$endgroup$













  • $begingroup$
    Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:26



















1












$begingroup$

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.






share|improve this answer











$endgroup$





















    -2












    $begingroup$

    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.






    share|improve this answer








    New contributor




    theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.






    $endgroup$









    • 2




      $begingroup$
      Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
      $endgroup$
      – Toby Speight
      13 hours ago











    Your Answer





    StackExchange.ifUsing("editor", function () {
    return StackExchange.using("mathjaxEditing", function () {
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    });
    });
    }, "mathjax-editing");

    StackExchange.ifUsing("editor", function () {
    StackExchange.using("externalEditor", function () {
    StackExchange.using("snippets", function () {
    StackExchange.snippets.init();
    });
    });
    }, "code-snippets");

    StackExchange.ready(function() {
    var channelOptions = {
    tags: "".split(" "),
    id: "196"
    };
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function() {
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled) {
    StackExchange.using("snippets", function() {
    createEditor();
    });
    }
    else {
    createEditor();
    }
    });

    function createEditor() {
    StackExchange.prepareEditor({
    heartbeatType: 'answer',
    autoActivateHeartbeat: false,
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader: {
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    },
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    });


    }
    });














    draft saved

    draft discarded


















    StackExchange.ready(
    function () {
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f1287%2fcalculating-compass-direction%23new-answer', 'question_page');
    }
    );

    Post as a guest















    Required, but never shown

























    7 Answers
    7






    active

    oldest

    votes








    7 Answers
    7






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes









    24












    $begingroup$

    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;
    }





    share|improve this answer











    $endgroup$









    • 9




      $begingroup$
      +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
      $endgroup$
      – k3b
      Mar 15 '11 at 9:53








    • 2




      $begingroup$
      The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:58












    • $begingroup$
      @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
      $endgroup$
      – munificent
      Mar 16 '11 at 1:44










    • $begingroup$
      Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
      $endgroup$
      – Danny Beckett
      Jan 5 '14 at 9:24
















    24












    $begingroup$

    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;
    }





    share|improve this answer











    $endgroup$









    • 9




      $begingroup$
      +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
      $endgroup$
      – k3b
      Mar 15 '11 at 9:53








    • 2




      $begingroup$
      The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:58












    • $begingroup$
      @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
      $endgroup$
      – munificent
      Mar 16 '11 at 1:44










    • $begingroup$
      Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
      $endgroup$
      – Danny Beckett
      Jan 5 '14 at 9:24














    24












    24








    24





    $begingroup$

    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;
    }





    share|improve this answer











    $endgroup$



    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;
    }






    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited May 19 '16 at 15:59

























    answered Mar 15 '11 at 2:17









    munificentmunificent

    1,24998




    1,24998








    • 9




      $begingroup$
      +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
      $endgroup$
      – k3b
      Mar 15 '11 at 9:53








    • 2




      $begingroup$
      The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:58












    • $begingroup$
      @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
      $endgroup$
      – munificent
      Mar 16 '11 at 1:44










    • $begingroup$
      Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
      $endgroup$
      – Danny Beckett
      Jan 5 '14 at 9:24














    • 9




      $begingroup$
      +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
      $endgroup$
      – k3b
      Mar 15 '11 at 9:53








    • 2




      $begingroup$
      The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:58












    • $begingroup$
      @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
      $endgroup$
      – munificent
      Mar 16 '11 at 1:44










    • $begingroup$
      Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
      $endgroup$
      – Danny Beckett
      Jan 5 '14 at 9:24








    9




    9




    $begingroup$
    +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
    $endgroup$
    – k3b
    Mar 15 '11 at 9:53






    $begingroup$
    +1 good and simple solution. Maybe its a good idea to replace the magic number ´45´ with ´int DegreesPerDirection = 360 / (directions.Length - 1)´ and ´23´ with ´DegreesPerDirection / 2´
    $endgroup$
    – k3b
    Mar 15 '11 at 9:53






    2




    2




    $begingroup$
    The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:58






    $begingroup$
    The index calculation seems solid, but a simple solution isn't a reuseable solution. I would still introduce an enum to represent the heading, as well as at least document the calculation, e.g. by removing the magic numbers as k3b mentions.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:58














    $begingroup$
    @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
    $endgroup$
    – munificent
    Mar 16 '11 at 1:44




    $begingroup$
    @k3b: Good call. @Steven: I'd introduce a Direction enum if there places where it was used. In this case all we're visibly using are degrees, so I'd leave it out. We can always add it later if we actually need it.
    $endgroup$
    – munificent
    Mar 16 '11 at 1:44












    $begingroup$
    Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
    $endgroup$
    – Danny Beckett
    Jan 5 '14 at 9:24




    $begingroup$
    Same calculation in JS, with more compass points: Is there a better way to get a compass point from 140° than a series of if's?
    $endgroup$
    – Danny Beckett
    Jan 5 '14 at 9:24













    7












    $begingroup$

    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().






    share|improve this answer











    $endgroup$









    • 1




      $begingroup$
      Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
      $endgroup$
      – Josh Earl
      Mar 16 '11 at 12:33
















    7












    $begingroup$

    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().






    share|improve this answer











    $endgroup$









    • 1




      $begingroup$
      Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
      $endgroup$
      – Josh Earl
      Mar 16 '11 at 12:33














    7












    7








    7





    $begingroup$

    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().






    share|improve this answer











    $endgroup$



    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().







    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited Mar 15 '11 at 4:10









    Mahesh Velaga

    1033




    1033










    answered Mar 14 '11 at 20:35









    Paulo PintoPaulo Pinto

    402210




    402210








    • 1




      $begingroup$
      Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
      $endgroup$
      – Josh Earl
      Mar 16 '11 at 12:33














    • 1




      $begingroup$
      Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
      $endgroup$
      – Josh Earl
      Mar 16 '11 at 12:33








    1




    1




    $begingroup$
    Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
    $endgroup$
    – Josh Earl
    Mar 16 '11 at 12:33




    $begingroup$
    Combining this with the original code makes for a more readable result than the shorter but more cryptic suggestion. In six months this will still make sense.
    $endgroup$
    – Josh Earl
    Mar 16 '11 at 12:33











    3












    $begingroup$

    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






    share|improve this answer











    $endgroup$









    • 1




      $begingroup$
      What use is the enum here? Why not simply map numbers to suffixes?
      $endgroup$
      – pdr
      Mar 14 '11 at 21:34






    • 1




      $begingroup$
      @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
      $endgroup$
      – Snowbear
      Mar 14 '11 at 21:37












    • $begingroup$
      @Snowbear, Good point
      $endgroup$
      – pdr
      Mar 15 '11 at 1:44










    • $begingroup$
      +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:05












    • $begingroup$
      @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
      $endgroup$
      – Snowbear
      Mar 15 '11 at 10:11
















    3












    $begingroup$

    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






    share|improve this answer











    $endgroup$









    • 1




      $begingroup$
      What use is the enum here? Why not simply map numbers to suffixes?
      $endgroup$
      – pdr
      Mar 14 '11 at 21:34






    • 1




      $begingroup$
      @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
      $endgroup$
      – Snowbear
      Mar 14 '11 at 21:37












    • $begingroup$
      @Snowbear, Good point
      $endgroup$
      – pdr
      Mar 15 '11 at 1:44










    • $begingroup$
      +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:05












    • $begingroup$
      @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
      $endgroup$
      – Snowbear
      Mar 15 '11 at 10:11














    3












    3








    3





    $begingroup$

    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






    share|improve this answer











    $endgroup$



    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







    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited Mar 15 '11 at 16:43

























    answered Mar 14 '11 at 20:47









    SnowbearSnowbear

    3,50311423




    3,50311423








    • 1




      $begingroup$
      What use is the enum here? Why not simply map numbers to suffixes?
      $endgroup$
      – pdr
      Mar 14 '11 at 21:34






    • 1




      $begingroup$
      @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
      $endgroup$
      – Snowbear
      Mar 14 '11 at 21:37












    • $begingroup$
      @Snowbear, Good point
      $endgroup$
      – pdr
      Mar 15 '11 at 1:44










    • $begingroup$
      +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:05












    • $begingroup$
      @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
      $endgroup$
      – Snowbear
      Mar 15 '11 at 10:11














    • 1




      $begingroup$
      What use is the enum here? Why not simply map numbers to suffixes?
      $endgroup$
      – pdr
      Mar 14 '11 at 21:34






    • 1




      $begingroup$
      @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
      $endgroup$
      – Snowbear
      Mar 14 '11 at 21:37












    • $begingroup$
      @Snowbear, Good point
      $endgroup$
      – pdr
      Mar 15 '11 at 1:44










    • $begingroup$
      +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:05












    • $begingroup$
      @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
      $endgroup$
      – Snowbear
      Mar 15 '11 at 10:11








    1




    1




    $begingroup$
    What use is the enum here? Why not simply map numbers to suffixes?
    $endgroup$
    – pdr
    Mar 14 '11 at 21:34




    $begingroup$
    What use is the enum here? Why not simply map numbers to suffixes?
    $endgroup$
    – pdr
    Mar 14 '11 at 21:34




    1




    1




    $begingroup$
    @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
    $endgroup$
    – Snowbear
    Mar 14 '11 at 21:37






    $begingroup$
    @pdr, it can be mapped directly to suffixes, but only if you are sure that you will never-never-never need the directions again. Otherwise leaving them as suffixes will lead to stringly-typed code.
    $endgroup$
    – Snowbear
    Mar 14 '11 at 21:37














    $begingroup$
    @Snowbear, Good point
    $endgroup$
    – pdr
    Mar 15 '11 at 1:44




    $begingroup$
    @Snowbear, Good point
    $endgroup$
    – pdr
    Mar 15 '11 at 1:44












    $begingroup$
    +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:05






    $begingroup$
    +1, the main point is mentioned here, separate logic from representation! The enum is a must. Also, if the class CompassDirectionAngle isn't to be used outside of the class (it's correctly set to private), I would use a Tuple<CompassDirection, int> array instead, so the class can be removed. Giving the array a proper name is sufficient enough, the class only adds a negligible bit of readability.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:05














    $begingroup$
    @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
    $endgroup$
    – Snowbear
    Mar 15 '11 at 10:11




    $begingroup$
    @Steven, I also thought about Tuples but they didn't fit into CR screen width :). But I agree, Tuples will look good here.
    $endgroup$
    – Snowbear
    Mar 15 '11 at 10:11











    2












    $begingroup$

    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.






    share|improve this answer











    $endgroup$













    • $begingroup$
      I don't really find an extension method appropriate here.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:29










    • $begingroup$
      @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
      $endgroup$
      – pdr
      Mar 15 '11 at 10:33










    • $begingroup$
      IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:46










    • $begingroup$
      @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:53










    • $begingroup$
      I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:54
















    2












    $begingroup$

    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.






    share|improve this answer











    $endgroup$













    • $begingroup$
      I don't really find an extension method appropriate here.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:29










    • $begingroup$
      @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
      $endgroup$
      – pdr
      Mar 15 '11 at 10:33










    • $begingroup$
      IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:46










    • $begingroup$
      @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:53










    • $begingroup$
      I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:54














    2












    2








    2





    $begingroup$

    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.






    share|improve this answer











    $endgroup$



    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.







    share|improve this answer














    share|improve this answer



    share|improve this answer








    edited Mar 15 '11 at 2:39

























    answered Mar 15 '11 at 1:42









    pdrpdr

    1,661139




    1,661139












    • $begingroup$
      I don't really find an extension method appropriate here.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:29










    • $begingroup$
      @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
      $endgroup$
      – pdr
      Mar 15 '11 at 10:33










    • $begingroup$
      IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:46










    • $begingroup$
      @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:53










    • $begingroup$
      I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:54


















    • $begingroup$
      I don't really find an extension method appropriate here.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:29










    • $begingroup$
      @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
      $endgroup$
      – pdr
      Mar 15 '11 at 10:33










    • $begingroup$
      IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
      $endgroup$
      – Steven Jeuris
      Mar 15 '11 at 10:46










    • $begingroup$
      @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:53










    • $begingroup$
      I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
      $endgroup$
      – pdr
      Mar 15 '11 at 10:54
















    $begingroup$
    I don't really find an extension method appropriate here.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:29




    $begingroup$
    I don't really find an extension method appropriate here.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:29












    $begingroup$
    @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
    $endgroup$
    – pdr
    Mar 15 '11 at 10:33




    $begingroup$
    @Steven, any reason for that? I find it makes the GetHeadingImage method more readable. But honestly, that's not the main part of the solution (static methods are equally effective); it's the maths vs lookup that is different from the other solutions (or was, at the time I wrote it).
    $endgroup$
    – pdr
    Mar 15 '11 at 10:33












    $begingroup$
    IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:46




    $begingroup$
    IMHO, I would expect such a method to be placed in a Compass class (or in java in the enum itself?), and be called from there. Otherwise the coupling just feels wrong. The list of extension methods for int would be infinite. Personally, I only use extension methods where I would otherwise have to write a helper class because I can't modify the source of the original class. Furthermore, the math is confusing, includes unnecessary magic numbers, and I believe is more complex than Snowbear's simple lookup.
    $endgroup$
    – Steven Jeuris
    Mar 15 '11 at 10:46












    $begingroup$
    @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
    $endgroup$
    – pdr
    Mar 15 '11 at 10:53




    $begingroup$
    @Steven, Ok, I disagree, but at least I understand now. Extension methods don't generally get out of hand because you limit access to them by namespace, and int is a perfect example of a class I have no access to while an enum can't have functionality. As for the magic numbers; I think Snowbear's are just hidden. Consider the directionAngles array written out in full. Any one of those numbers could be wrong and cause a subtle bug.
    $endgroup$
    – pdr
    Mar 15 '11 at 10:53












    $begingroup$
    I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
    $endgroup$
    – pdr
    Mar 15 '11 at 10:54




    $begingroup$
    I should point out here (in the midst of seeming criticism) that I voted up Snowbear's answer myself. I prefer it to the others by a long shot.
    $endgroup$
    – pdr
    Mar 15 '11 at 10:54











    2












    $begingroup$

    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.






    share|improve this answer









    $endgroup$













    • $begingroup$
      Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
      $endgroup$
      – Leonid
      Aug 9 '12 at 21:26
















    2












    $begingroup$

    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.






    share|improve this answer









    $endgroup$













    • $begingroup$
      Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
      $endgroup$
      – Leonid
      Aug 9 '12 at 21:26














    2












    2








    2





    $begingroup$

    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.






    share|improve this answer









    $endgroup$



    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.







    share|improve this answer












    share|improve this answer



    share|improve this answer










    answered Aug 9 '12 at 18:16









    HightechriderHightechrider

    1213




    1213












    • $begingroup$
      Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
      $endgroup$
      – Leonid
      Aug 9 '12 at 21:26


















    • $begingroup$
      Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
      $endgroup$
      – Leonid
      Aug 9 '12 at 21:26
















    $begingroup$
    Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:26




    $begingroup$
    Hey! This code borrows heavily from my patented FizzBuzz algorithm implementation. My lawyer will get in touch with you.
    $endgroup$
    – Leonid
    Aug 9 '12 at 21:26











    1












    $begingroup$

    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.






    share|improve this answer











    $endgroup$


















      1












      $begingroup$

      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.






      share|improve this answer











      $endgroup$
















        1












        1








        1





        $begingroup$

        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.






        share|improve this answer











        $endgroup$



        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.







        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited Mar 15 '11 at 5:11

























        answered Mar 15 '11 at 3:58









        JustinCJustinC

        1114




        1114























            -2












            $begingroup$

            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.






            share|improve this answer








            New contributor




            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            $endgroup$









            • 2




              $begingroup$
              Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
              $endgroup$
              – Toby Speight
              13 hours ago
















            -2












            $begingroup$

            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.






            share|improve this answer








            New contributor




            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            $endgroup$









            • 2




              $begingroup$
              Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
              $endgroup$
              – Toby Speight
              13 hours ago














            -2












            -2








            -2





            $begingroup$

            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.






            share|improve this answer








            New contributor




            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            $endgroup$



            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.







            share|improve this answer








            New contributor




            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            share|improve this answer



            share|improve this answer






            New contributor




            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            answered 14 hours ago









            theodore panagostheodore panagos

            1




            1




            New contributor




            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.





            New contributor





            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            theodore panagos is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.








            • 2




              $begingroup$
              Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
              $endgroup$
              – Toby Speight
              13 hours ago














            • 2




              $begingroup$
              Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
              $endgroup$
              – Toby Speight
              13 hours ago








            2




            2




            $begingroup$
            Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
            $endgroup$
            – Toby Speight
            13 hours ago




            $begingroup$
            Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. (Note: some of the existing answers are very old, and don't represent current best practice for answering!)
            $endgroup$
            – Toby Speight
            13 hours ago


















            draft saved

            draft discarded




















































            Thanks for contributing an answer to Code Review Stack Exchange!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid



            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.


            Use MathJax to format equations. MathJax reference.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f1287%2fcalculating-compass-direction%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            Popular posts from this blog

            404 Error Contact Form 7 ajax form submitting

            How to know if a Active Directory user can login interactively

            TypeError: fit_transform() missing 1 required positional argument: 'X'