apple / swift-numerics

Advanced mathematical types and functions for Swift
Apache License 2.0
1.68k stars 142 forks source link

GCD function for BinaryInteger #189

Closed NevinBR closed 3 years ago

NevinBR commented 3 years ago

Added a gcd static method to BinaryInteger.

(Edit: gcd is now a free function.)

The initial implementation uses the binary GCD algorithm.

I believe that gcd is a sufficiently widespread term of art to prefer it over a more verbose spelling. However if others disagree, we could use greatestCommonDivisor instead.

stephentyrone commented 3 years ago

One higher-level question; should we expose a gcd operation on > 2 integers, or on collections? I'm not sure how valuable a variadic gcd would be (since there's not a great way to marshal multiple arguments like that), but is it worth having a more convenient spelling than reduce(into: 0) { $0 = gcd($0, $1) }? I think it might be worth providing because it isn't immediately obvious to most users that they can provide 0 as an initial value for the reduction.

NevinBR commented 3 years ago

One higher-level question; should we expose a gcd operation on > 2 integers, or on collections?

That’s a good question. We should probably hash out things like that over on issue #10.

stephentyrone commented 3 years ago

We should probably hash out things like that over on issue #10.

I'm not sure #10 is the right place to discuss it, but I'm perfectly happy to punt it to another PR; it would clearly be built on top of this anyway.

NevinBR commented 3 years ago

I'm not sure #10 is the right place to discuss it, but I'm perfectly happy to punt it to another PR; it would clearly be built on top of this anyway.

I was thinking along the lines of discussing the scope of the integer utilities modules and general shape of its APIs.

One thing I’ve found with Github is that it doesn’t really provide an obvious location for discussion, planning, and questions about a project.

stephentyrone commented 3 years ago

@swift-ci test

stephentyrone commented 3 years ago

Thanks @NevinBR! Do you want to do anything further with this before I merge it?

stephentyrone commented 3 years ago

@swift-ci test

stephentyrone commented 3 years ago

Thanks for your patience, @NevinBR!