Returning a different result depending on which of N values is the smallest

Question

Not sure how to make the title more descriptive, so I'll just start with an example. I'm using the bit of code below which selects a direction from an enum, depending on which of the four axis form the smallest angle in comparison with a given direction.

static Direction VectorToDirection(Vector2 direction)
{
    double upDiff = System.Math.Acos(Vector2.Dot(direction, -Vector2.UnitY));
    double downDiff = System.Math.Acos(Vector2.Dot(direction, Vector2.UnitY));
    double leftDiff = System.Math.Acos(Vector2.Dot(direction, -Vector2.UnitX));
    double rightDiff = System.Math.Acos(Vector2.Dot(direction, Vector2.UnitX));

    double smallest = System.Math.Min(System.Math.Min(upDiff, downDiff), System.Math.Min(leftDiff, rightDiff));

    // This is the part I'm unsure about i.e.
    // Comparing smallest with each value in turn
    // To find out which of the four was "selected"
    if (smallest == upDiff) return Direction.Up;
    if (smallest == downDiff) return Direction.Down;
    if (smallest == leftDiff) return Direction.Left;
    return Direction.Right;
}

But I get the Resharper warning about floating point equality at the end. I'm guessing it should not be a problem due to the implementation of Min, but was wondering if there might be a better idiom to solve this kind of problem besides comparing smallest with each of the original values.

Thank you for all the answers, I was a bit unsure which one to choose. For a small number of values such as this, comparing them manually like Brandon wrote seems like a better choice than what I did. For the general case, I like Alexei's approach of finding the minimum by index, and marked that as the answer since it's broader. — David Gouveia, Aug 06 '12 at 18:55

score 2 · Answer 1 · answered Aug 06 '12 at 18:02

2

This code should get you the desired result.

    if ((Math.Abs(direction.x) >= Math.Abs(direction.y))
      return direction.x >= 0 ? Direction.Right : Direction.Left;
    return direction.y >= 0 ? Direction.Up : Direction.Down;

answered Aug 06 '12 at 18:02

mrranstrom

228
2
10

Note that like your original code, this does not handle edge cases specially. In your code, precedence for ties is given in the order Up Down Left Right, and in my code it is given in the order Right Left Up Down. – mrranstrom Aug 06 '12 at 18:23
Thanks! While not answering my question directly because that code snippet was just an example of the broader problem, it does provide a much simpler and efficient implementation to this specific case, and I've already made the change in my code base :) – David Gouveia Aug 06 '12 at 18:39

score 1 · Answer 2 · edited May 23 '17 at 12:20

1

You could make a dictionary of <double ,Direction> sort the dictionary, and get the smallest value with the right enum to return.

edited May 23 '17 at 12:20

Community

1
1

answered Aug 06 '12 at 17:59

Yochai Timmer

48,127
24
147
185

score 1 · Answer 3 · answered Aug 06 '12 at 18:03

You could define a class that contains the diff and the value associated with it. Then you make a collection out of these objects and sort them by the diff. After which you return the value associated with the first element.

However, I wouldn't go there in your case, the code is clear as it is. If the amount of possible values was a lot larger (or not known in advance), only then I would go for a more generic solution.

score 1 · Answer 4 · answered Aug 06 '12 at 18:05

1

Can you just write some if statements?

if  (upDiff < leftDiff && upDiff < downDiff && upDiff < rightDiff) return Direction.Up;
if  (leftDiff < upDiff && leftDiff < downDiff && leftDiff < rightDiff) return Direction.Left;
if  (rightDiff < leftDiff && rightDiff < upDiff && rightDiff < downDiff) return Direction.Right;
return Direction.Down;

Maybe it can be cleaned up further, but this seems straight forward.

answered Aug 06 '12 at 18:05

Brandon

983
6
15

This seems so obvious, but for a case like this where there aren't many values, this does strike me as a much better solution than dealing with `Min` and equality comparisons. – David Gouveia Aug 06 '12 at 18:42

score 1 · Accepted Answer · answered Aug 06 '12 at 18:09

I would put all choices in an array and find min index. For 4 choices sorting is likely overkill. If performance of this code is important - make sure to measure time for different variants.

Non-compiled code below:

static Direction VectorToDirection(Vector2 direction)
{
  var directions = new Direction[]{
    Direction.Up, Direction.Down, Direction.Right, Direction.Left };
  var unit = new Vector2[] {
   -Vector2.UnitY, Vector2.UnitY, Vector2.UnitX,-Vector2.UnitY};

  var minAngle = 10;
  var minIndex = -1;
  for(var index = 0; index < directions.length; index++)
  {
    double diff = System.Math.Acos(Vector2.Dot(direction, unit[index]));
    if (diff < minAngle)
    { 
      minAngle = diff;
      minIndex = index;
    }

  return directions[minIndex];
}

Seems like the best solution for the general case where there are a large or variable number of values. — David Gouveia, Aug 06 '12 at 18:49

Sebastian Graf · Answer 6 · 2012-08-06T19:18:55.403

1

static Direction VectorToDirection(Vector2 direction)
{
    var mappings = new[]
    {
        new { Direction = Direction.Up, Axis = -Vector2.UnitY },
        new { Direction = Direction.Down, Axis = Vector2.UnitY },
        new { Direction = Direction.Left, Axis = -Vector2.UnitX },
        new { Direction = Direction.Right, Axis = Vector2.UnitX }
    };
    return mappings.OrderBy(m => Math.Acos(Vector2.Dot(direction, m.Axis))).Select(m => m.Direction).First();
}

The Linq way. This is not tested, but you should get it.

edited Aug 06 '12 at 19:18

answered Aug 06 '12 at 18:09

Sebastian Graf

3,602
3
27
38

This is interesting, and it did work (except you forgot to add the `Acos`). I was wondering if there is a way to make `mappings` static to avoid recreating it every time though. Because I can't use `var` outside of a method, and I couldn't figure out what what type to choose. :) – David Gouveia Aug 06 '12 at 18:33
It's actually an anonymus type... but you could easily use Tuple[] (or a custom POCO for that matter) as a static readonly instead, sacrificing some readility. Notice that OrderBy uses deferred execution, so when only getting the first element, it doesn't have to order the whole enumerable, but rather find the Min value... which is exactly what you wanted. – Sebastian Graf Aug 06 '12 at 19:15
@DavidGouveia: Added Math.Acos – Sebastian Graf Aug 06 '12 at 19:19
Sorry to spam, but my edit time expired. I hit [this](http://code.logos.com/blog/2010/04/a_truly_lazy_orderby_in_linq.html), so OrderBy isn't that deferred as it seems. – Sebastian Graf Aug 06 '12 at 19:35

score 0 · Answer 7 · answered Aug 06 '12 at 17:56

0

set inaccuracy

if ((Math.Abs(smallest - upDiff) < 0.00001) return Direction.Up;

answered Aug 06 '12 at 17:56

burning_LEGION

13,246
8
40
52

2

I don't think it applies in this particular case - the value would match exactly to one of the choices as double is used more like enum. The suggestion will resolve resharper warning, but adds unnecessary code. – Alexei Levenkov Aug 06 '12 at 17:59

score 0 · Answer 8 · answered Aug 06 '12 at 18:01

0

With you are doing, I do not think there would be any errors. However, if you end up refactoring or changing the code in the future, you could run into some problems.

To be safe, just do what resharper suggests.

answered Aug 06 '12 at 18:01

Colin D

5,641
1
23
35

score 0 · Answer 9 · answered Aug 06 '12 at 18:03

0

I usually use switch instead of if-else (atleast 3 clauses) - somewhat concise and faster.

switch(smallest) {
  case upDiff: return Direction.Up;
  case downDiff: return Direction.Down;
  case leftDiff: return Direction.Left;
  default: return Direction.Right; 
}

answered Aug 06 '12 at 18:03

Channs

2,091
1
15
20

`smallest` is not an integral type so this option is out :) – David Gouveia Aug 06 '12 at 18:19

Returning a different result depending on which of N values is the smallest

9 Answers9

Linked