Quick code scan feedback

Jul 28, 2008 at 8:38 PM
Edited Jul 28, 2008 at 8:40 PM
Hey codekaizen,

As you reported on my blog, we both published a .NET Money implementation on exactly the same day. I’m still amazed! ;)

I did a quick scan of your implementation this weekend. I really like the completeness of your code. The completeness of the type converters and the implementation of IFormattable.  Also, your test suite looks nice and complete.

By your comment on my implementation, you also got me to think and do some extra research on the best way to store monetary values and the best way to do calculations with them. Reading more, I got to think that it might indeed be wise to follow your path and use a two integers two represent the value (one for the whole number and the other for that fraction). In essence, by using an int64 for the whole number and an int32 for the fraction, you’ve created a fixed point 96 bit decimal. I’m using the regular Decimal type, which is a floating point type. I’m still not sure, however, if it being a floating point type is such a bad thing. First of all, the decimal point can be fixed per object instance. Second, if you need the extra precision, you can have it with the same type. Literature (like this from IBM) suggest a theoretic need for 2191(!) decimal places, but settles for 25-30 decimal places being enough for almost all cases. With the Decimal type you can at least reach a precision of 28 decimal places, but with the two-integer solution you’re always stuck with a maximum precision of 9 decimal places.

Reading all this, it became clear to me that I got some arithmetic sorting out to do with my implementation, which mainly focuses on easy creation and storage of monetary values. I’m going to puzzle a little more on this for my version 2 implementation.

Anyway, one last thing: I’m not sure your approach of adding the currency to the monetary value is “inviting” enough. The way a user of your type needs to address a certain currency code seems a little verbose (mainly judging from your tests). Although, in all honesty, I have not gotten around to dive deeply into that yet.

Keep up the good work!

Pascal.


Aug 20, 2008 at 12:57 PM
Edited Aug 21, 2008 at 3:44 PM
Hi there codekaizen - a couple of thoughts.

What you say about System.Decimal isn't really accurate - it's a good type for representing Money and has the advantage of easily mapping to a database. Take a look here for a full explanation. How does this money class map to the database?

EDIT: I see, you have a range of ToXxx() methods, cool.

Names for simple enums shouldn't be pluralised.

You really need the mainline usage code samples on the project home page. Few people have the time it takes to look at the source code to initially evaluate this (including me!).

Thanks! Pete.