Fixing the euclid alg to allow for negative inputs

[Gathros]

No description provided.

[ghost]

This seems like an unnecessary change. This isn't meant to be production code but example code for people to try and understand the concepts and by adding the abs() it lengthens the code / makes it slightly harder to read.

[june128]

I'am on your side there. I think we have to mark the lines of code for handling the negative values clearly or provide two versions of the functions.

[leios]

Yeah, I think adding an abs() doesn't make things too confusing and serves a purpose. I do agree with KingFredrickVI in general, though. Code cleanliness is most important here.

I think I'll merge these two in this case.

[zsparal]

@kingfredrickvi Just to clarify something since we talked about this in length over on Discord: because of some C/C++ idiosyncrasies the behavior of the one without abs was actually implementation-defined:

int main() {
  return -10 % -7;
}

The compiler is free to translate this to the following:

int main() {
  return 0;
}

I think avoiding hard to debug and non-obvious mistakes is a nice goal in a public algorithm collection.

[HappyLittleGit]

I agree with unnecessary change. Example code is best kept as clean and concise as possible. abs should do the trick.

[strega-nil]

@Gustorn I don't believe that's true of any modern versions of C or C++.

Also, implementation defined is quite different from unspecified (is quite different from undefined). Implementation defined means it must follow a specific set of rules - in this case:

When integers are divided and the division is inexact, if both operands are positive the result of the / operator is the largest integer less than the algebraic quotient and the result of the % operator is positive. If either operand is negative, whether the result of the / operator is the largest integer less than the algebraic quotient or the smallest integer greater than the algebraic quotient is implementation-defined, as is the sign of the result of the % operator. If the quotient a/b is representable, the expression (a/b)*b + a%b shall equal a.

(C89 draft standard)

Let's go through it with your example - -10 % -7

Well, the two valid answers to the question -10 / -7 are: 1 or 2. There are no other valid answers.

1 is the largest integer less than the algebraic quotient (approximately 1.42), and 2 is the smallest integer greater than the algebraic quotient (again, around 1.42).

Let's look at the equation for finding the remainder operator, given q = -10 / -7, and r = -10 % -7:

r * -7 + q = -10

r * -7 + 10 = -q

r * 7 - 10 = q

given r = 1, q = 1 * 7 - 10 = -3

given r = 2, q = 2 * 7 - 10 = 14 - 10 = 4

Therefore, the only two valid answers to -10 % -7 are -3, or 4. There are no other valid answers under C89, or C++98/03.

C++11, C99, and later standards fix this, so that integer division must round towards zero - thus, -10 / -7 always equals 1, and -10 % -7 equals -3.

side note: this is why I called the Rust version euclid_rem - it's not the modulus operator, it's the remainder operator.

[zsparal]

@ubsan You are right, for some reason I thought the return value would be unspecified even though I rembered that it was implementation-defined. (and yes, I know the difference between undefined/unspecified/implementation-defined).

Unfortunately it still has the ability to mess with the algorithm without the abs call.

[strega-nil]

@Gustorn yeah, but in a completely defined way 😄

I'm assuming C99/C++11