tags:

views:

432

answers:

4

Hi,

I'm working on a simple rotate routine which normalizes an objects rotation between 0 and 360 degrees. My C# code seems to be working but I'm not entirely happy with it. Can anyone improve on the code below making it a bit more robust?

public void Rotate(int degrees)
    {
        this.orientation += degrees;

        if (this.orientation < 0)
        {
            while (this.orientation < 0)
            {
                this.orientation += 360;
            }
        }
        else if (this.orientation >= 360)
        {
            while (this.orientation >= 360)
            {
                this.orientation -= 360;
            }
        }
    }
+8  A: 

Use modulo arithmetic:

this.orientation += degrees;

this.orientation = this.orientation % 360;

if (this.orientation < 0)
{
    this.orientation += 360;
}
tvanfosson
Can you get a negative result from modulous? Floating point issue?
Swanny
C gives you negatives, I'm assuming C# does the same. And, since degrees is an int, you're limited to integer rotations. It's not explicitly spelled out that orientation is an int but it would be strange if it wasn't.
paxdiablo
Oh. So you can. -1 % 360 is negative 1. I expected 359. Silly me. You learn something every day.
Swanny
+5  A: 

This can be simplified to the following.

public void Rotate (int degrees) {
    this.orientation = (this.orientation + degrees) % 360;
    if (this.orientation < 0) this.orientation += 360;
}

C# follows the same rules as C and C++ and i % 360 will give you a value between -359 and 359 for any integer, then the second line is to ensure it's in the range 0 through 359 inclusive.

If you wanted to be shifty, you could get it down to one line:

    this.orientation = (this.orientation + (degrees % 360) + 360) % 360;

which would keep it positive under all conditions but that's a nasty hack for saving one line of code, so I wouldn't do it, but I will explain it.

From degrees % 360 you will get a number between -359 and 359. Adding 360 will modify the range to between 1 and 729. If orientation is already positive, adding this will guarantee it still is, and the final % 360 will bring it back to the range 0 through 359.

At a bare minimum, you could simplify your code since the ifs and whiles can be combined. For example, the result of the conditions in these two lines:

if (this.orientation < 0)
while (this.orientation < 0)

is always the same, hence you don't need the surrounding if.

So, to that end, you could do:

public void Rotate (int degrees) {
    this.orientation += degrees;
    while (this.orientation <   0) this.orientation += 360;
    while (this.orientation > 359) this.orientation -= 360;
}

but I'd still go for the modulus version since it avoids loops. This will be important when a user enters 360,000,000,000 for the rotation (and they will do this, believe me) and then find they have to take an early lunch while your code grinds away :-)

paxdiablo
I´d say `this.orientation = (this.orientation + degrees + 360) % 360;` is easier to read.
jensgram
@jensgram, the problem with that one is that it can *still* give you a number in the range -359 through -1 if, for example, orientation is 0 and degrees is -719, giving -359. You have to normalize degrees first then force it to positive.
paxdiablo
@paxdiablo, you're right! I didn't anticipate `degrees` to be outside +/-359.
jensgram
+2  A: 

I sort of quickly mocked this up in AS3, but should work (you may need += on the angle)

private Number clampAngle(Number angle)
{
 return (angle % 360) + (angle < 0 ? 360 : 0);
}
+1  A: 

This is one that normalizes to any range. Useful for normalizing between [-180,180], [0,180] or [0,360].

( it's in C++ though )

//Normalizes any number to an arbitrary range 
//by assuming the range wraps around when going below min or above max 
double normalise( const double value, const double start, const double end ) 
{
  const double width       = end - start   ;   // 
  const double offsetValue = value - start ;   // value relative to 0

  return ( offsetValue - ( floor( offsetValue / width ) * width ) ) + start ;
  // + start to reset back to start of original range
}

For ints

//Normalizes any number to an arbitrary range 
//by assuming the range wraps around when going below min or above max 
int normalise( const int value, const int start, const int end ) 
{
  const int width       = end - start   ;   // 
  const int offsetValue = value - start ;   // value relative to 0

  return ( offsetValue - ( ( offsetValue / width ) * width ) ) + start ;
  // + start to reset back to start of original range
}

So basically the same but without the floor. The version I personally use is a generic one that works for all numeric types and it also uses a redefined floor that does nothing in case of integral types.

QBziZ