arkworks-rs / algebra

Libraries for finite field, elliptic curve, and polynomial arithmetic
https://arkworks.rs
Apache License 2.0
633 stars 247 forks source link

Use `in_place` suffix for `BigInteger` operations #781

Open tcoratger opened 8 months ago

tcoratger commented 8 months ago

In the whole codebase, there is a standard which is to include the suffix in_place on operations which modify the value passed as a mutable parameter. For example here:

https://github.com/arkworks-rs/algebra/blob/cc2ad8c368c323ee9d9aacaabf8e353de95682e6/ec/src/models/bn/g2.rs#L53-L77

We know from the function name that the self mutable will be modified.

However similar operations are performed in the BigInteger trait like:

https://github.com/arkworks-rs/algebra/blob/cc2ad8c368c323ee9d9aacaabf8e353de95682e6/ff/src/biginteger/mod.rs#L1062

or

https://github.com/arkworks-rs/algebra/blob/cc2ad8c368c323ee9d9aacaabf8e353de95682e6/ff/src/biginteger/mod.rs#L1037

without including the in_place suffix. For readability and consistency, it could be nice to include this suffix for all the BigInteger operations concerned by this type of logic.

Pratyush commented 8 months ago

Yes, this would be great!