m3m0ry / fixedpoint

Decimal fixed point type
Boost Software License 1.0
6 stars 3 forks source link

improvement: use core.checkedint to check overflow? #4

Open mw66 opened 4 years ago

mw66 commented 4 years ago

e.g.

https://github.com/qznc/d-money/blob/master/source/money.d#L57

import core.checkedint : adds, subs, muls, negs;

https://github.com/qznc/d-money/blob/master/source/money.d#L135

            auto ret = fromLong(adds(amount, rhs.amount, overflow));

sigh, we are re-invent the wheels all the time.

m3m0ry commented 4 years ago

I don't try to re-invent the wheel. This fixedpoint type is supposed to be simple, yet fast. Handling the normal arithmetic types also does not produce an OverflowException: example.

I was thinking about another improvment though. The current version has long as the storing type. If I template it, you could use BigInt in order to have more precision without an overflow.

mw66 commented 4 years ago

Handling the normal arithmetic types also does not produce an OverflowException: example.

I think you misunderstood what I mean: we want the overflow be detected, and the OverflowException be thrown and catch-ed:

the example program silently overflowed, without letting the caller know, and fix it:

...
1073741824
-2147483648
0
0
0
...

e.g. oh, the value overflowed in the loop, so the value is no longer trust-able, let's stop the loop.

void main()
{
    int a = 1;
    foreach(b; 1..100)
    {
      try {
        a *= 2;
        writeln(a);
      } catch (OverflowException ex) {
        // a is not trust-able anymore, let's break
        break;
      }
    }
}
mw66 commented 4 years ago

That's why I'm saying we want use the

import core.checkedint : adds, subs, muls, negs;

as the other package do.

m3m0ry commented 4 years ago

I fully understood what you mean. I understand the reason of wanting to throw OverflowExceptions. However my intention for this package was to have a dead simple value type, which would behave similarly to types like int, long and so on. They don't throw exceptions, because it the generated code wouldn't be very fast.

However I also wanted to extend this package with more types. A binary fixed point. An arbitrary fixed point.

I am currently reading into the decimal floating point, which I will try to complete and maintain.

The whole thematic regarding fixed-point, and decimal seems to be a little confusing. It is also one thing that D truly lacks of right now.

I don't know what your project is, and what exact type you would need. I am playing with the idea to do all of them, but I would love some help. At least with idea exchange and code review. If you are interested, we could skype: thunder-memory (or discord, or teamspeak, or anything you use and I also have :) )

schveiguy commented 4 years ago

I think the template could be extended to allow alternate base types instead of long. This way you could use an integer type that checks for overflows (such as Checked).

For example Fixed!(4, Checked!long) could be made to do the right thing.

m3m0ry commented 4 years ago

Did not know about Checked. I planned to do the template anyway, so user can set the value type, but with Checked that makes even more sense.

m3m0ry commented 4 years ago

I have implemented the template. I need to incorporate now some more tests and probably do some benchmarks, i.e. slapping @safe and nothrow on these bad boys before DIPs make it default anyway :laughing:

schveiguy commented 4 years ago

You don’t need to annotate templates. @safe and nothrow will be inferred from the code compiled.

mw66 commented 4 years ago

However my intention for this package was to have a dead simple value type, which would

OK, as long as there's no misunderstanding, that's fine. And I think you are right, we probably need several implementations of the fixed point decimal type, and the user can choose based on his/her usage requirement: from the most rudimentary unchecked dead simple value type, all the way to the fully-checked, arbitrary length decimals. The implementation can use either the same struct with different template config parameters, or separate structs (I think the arbitrary length one certainly will be separate struct).

If you are interested, we could skype: thunder-memory (or discord, or teamspeak, or anything

Thanks for the invitation, but I don't use any IM. We can just talk here, or on forum.dlang.org. BTW, I think @schveiguy has much more experience in D than me, I'm a new comer to D.

m3m0ry commented 4 years ago

If I have the time today, I will implement some more mature testing. Then you could use Checked!int as a template parameter. The behavior is already implemented, it just needs more testing, so I can bump the version of this package.

@schveiguy seems to have more experience and he gave good insight before, however this does not grow the D community. Even using this package and creating issues like this help.

mw66 commented 4 years ago

FYI:

checkedint 0.7.0 Checked integer math types and operations.

https://code.dlang.org/packages/checkedint

Just saw it, I never used it.

mw66 commented 4 years ago

@m3m0ry

Another interesting idea: storing numbers as ratios, i.e. pair of integers. E.g. I bought 3 apples with $1, each apple cost 1/3 (not 0.33333....) is stored as 1/3, not rounded in any way:

https://wukix.com/lisp-decimals#theory

m3m0ry commented 4 years ago

You mean the rational data type. Yes this is also not present in Phobos, as far as I know. I also know it from common lisp.

I have not found it on dub. That would be a cool first project, hint, hint :) And if you reproduce it from lisp, then you have the option to "steal" all the tests.

Regarding fixedpoint, I hope I can find some time for creating tests on Saturday or Monday.

schveiguy commented 4 years ago

I use this in my code. It’s not maintained but hasn’t caused me any issues. https://code.dlang.org/packages/rationald