nim-lang / bigints

BigInts for Nim
MIT License
124 stars 32 forks source link

Add toBytes proc #128

Open ehmry opened 1 year ago

ehmry commented 1 year ago
dlesnoff commented 1 year ago

Thanks for your contribution! The algorithm looks fine to me. – Could you add tests for the toBytes procedure? – Could you add the corresponding fromBytes procedure? I believe that the tliterals was renamed literals as to not run these tests. You might want to check the log for this specific file.

ehmry commented 1 year ago

I­ pushed a test for toBytes. I will push fromBytes soon.

ehmry commented 1 year ago

Now with fromBytes and a test of random serializations.

konsumlamm commented 1 year ago

I'm against exporting isNegative and isZero. I think the better solution would be to support comparing with ints, then we can just use < 0 and == 0 (see #132).

ehmry commented 1 year ago

I'm against exporting isNegative and isZero. I think the better solution would be to support comparing with ints, then we can just use < 0 and == 0 (see #132).

That would be adding more code to do something that is already implemented.

konsumlamm commented 1 year ago

I'm against exporting isNegative and isZero. I think the better solution would be to support comparing with ints, then we can just use < 0 and == 0 (see #132).

That would be adding more code to do something that is already implemented.

Yes, but it's also uglier imo.

dlesnoff commented 1 year ago

I am for exporting isNegative only and implement equality with integers. Comparing with integers and fetching a boolean flag are two different things.

konsumlamm commented 1 year ago

Note that isNegative really means "is less than or equal to zero", as 0 can also have isNegative set.

dlesnoff commented 1 year ago

Thanks for the reminder.