nim-lang / bigints

BigInts for Nim
MIT License
124 stars 32 forks source link

Unify `initBigInt` for `SomeInteger`s #142

Open konsumlamm opened 11 months ago

konsumlamm commented 11 months ago

Unify the various initBigInt versions that convert an integer to a BigInt into one func.

pietroppeter commented 11 months ago

FWIW I do prefer the original version and I am not able to see advantages of the unification.

konsumlamm commented 11 months ago

Before, initBigInt for int and uint were templates, so it wasn't possible to e.g. pass them to higher order functions. Additionally, now you can specify the integer type in all cases. I think it's more consistent to have one function for all integer types than have some with a generic argument and some without (and some templates). This also reduced the number of cases (there is no extra case needed for int and uint anymore).

pietroppeter commented 11 months ago

Ok, thanks for the explanation

dlesnoff commented 11 months ago

I had the same impression as @pietroppeter and I also noticed that it greatly simplifies the documentation with just what we want in the public API. Are you just sure that it was not purposefully chosen to use templates there in the first place? EDIT: I performed a Git blame, and I think that it is nice to remove these.