Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expected behaviour of Radians roll-around? #40

Closed
thomthom opened this issue Feb 23, 2018 · 4 comments
Closed

Expected behaviour of Radians roll-around? #40

thomthom opened this issue Feb 23, 2018 · 4 comments

Comments

@thomthom
Copy link
Contributor

I was setting up tests for the current behaviour or Radians:

Radians::Radians(const double &rhs):
m_val(rhs)
{
if(rhs > (2*PI)) {
int numPi = rhs / (2*PI); // Get number of times 2pi fits in to rhs
m_val = rhs - (numPi*2*PI);
}
else if(rhs < 0.0) {
int numPi = (rhs / (2*PI)) - 1; // Get number of times 2pi fits in to rhs
m_val = rhs - (numPi*2*PI);
}
}

From what I understand of the code it's trying to cap the value by rolling around 2*PI.

For 7.0 I get 0.71681469282041377 which I'd expect.
For -7.0 I get 5.5663706143591725 which I did not expect.

I would have thought I should get -0.71681469282041377.

Am I wrong? Can you explain the intent so I can fill in test unit for this?

@thomthom
Copy link
Contributor Author

This version returns the result I originally expected:

Radians::Radians(const double &rhs):
  m_val(rhs)
{
  if(rhs > (2 * PI)) {
    m_val = std::fmod(rhs, 2 * PI);
  }
  else if(rhs < 0.0) {
    m_val = std::fmod(rhs, -(2 * PI));
  }
}

@TommyKaneko
Copy link
Owner

So Radians always produces a positive number between 0 and 2*PI. A negative number will be converted to its positive equivalent. So 5.5663706143591725 is the same as -0.71681469282041377

On a side note - I think there is mixing of types - ie 2*PI should be 2.0*PI on line 50, 52 and 56.

thomthom added a commit to thomthom/Sketchup-API-C-Wrapper that referenced this issue Feb 23, 2018
@thomthom
Copy link
Contributor Author

So Radians always produces a positive number between 0 and 2*PI. A negative number will be converted to its positive equivalent. So 5.5663706143591725 is the same as -0.71681469282041377.

Gotcha. I'll correct my test and implementation.

@thomthom
Copy link
Contributor Author

Corrected in 89089d2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants