automatevalue / nodamoney Goto Github PK

Hi!
In most cases base currency have bigger value than quote currency.
But in some situations (money order between two people with different currencies; or simply nonstandard pair where quote currency and base currency are overturned) ExchangeRate.Value is much less than 1 and loss of fraction occurs.

Maybe rounding should be more than 4 digits or to add exception if ExchangeRate.Value is less than 1 to prevent losses.

ExchangeRate can now only be initiated, like:
var fx = new ExchangeRate(Currency.FromCode("EUR"), Currency.FromCode("USD"), 1.2591)

Add the option to use currency codes to initialize an ExchangeRate:
var fx = new ExchangeRate("EUR", "USD", 1.2591)

When currency is inferred from the invariant culture, which has XDR as the currency code, ToString() throws because it's trying to setup a NumberFormatInfo with -1 DecimalDigits.

ToString() shouldn't throw, as per: https://docs.microsoft.com/en-us/dotnet/api/system.object.tostring?view=netcore-3.1#notes-to-inheritors

Either don't allow Money to be created with XDR, or use a workable value for DecimalDigits in CurrencyRegistry.

This will likely cause a problem for developers who don't need multicurrency support, but have a CI process using dotnet core SDK docker images where the default culture is the invariant culture.

As a side note, Unicode chose to use 2dp, even though ISO publication is N/A. Practically XDR probably needs decimal places. https://www.unicode.org/cldr/charts/latest/supplemental/detailed_territory_currency_information.html

If I try something like this

Money money = new Money(10, "GBP");
list[gbpAnountNdx] += 10;

I do not end up with 20 GBP, I end up with the exception.
Reson? My local culture is USA, so plus operator first created sctruct Money using "USD" and then tried to perform addition, which caused exception since I attempted to add two different currencies.

Can we introduce addition operator override that would treat numbers in a proper way - look at currency, create new Money with that currency and then call existing override that sums up two Money structs?

By default the Money instance will round every math operation (+. -, *. /, etc) that might have fractions of cents using a RoundingMode.

This will cause issues with multiple operations and money loss. If you multiple by a couple of numbers, do some division, add a few things, do some more division, you'll end up losing multiple cents along the way. This happens frequently in financial applications and many others as well.

There needs to be a way to perform multiple calculations on a Money instance without any rounding and then round at the end. Perhaps a MoneyCalculater (or building pattern) of some sort that can provide multiple operations without rounding and in the end provide back a Money instance that is rounded.

The best is probably to use a delegate like Func<Money, Money>, Action<Money> or a custom delegate.

See an example like https://github.com/gigs2go/joda-money-calculator/tree/master/joda-money-calculator or the builder pattern in JavaMoney.

At the moment, ToString formats money using its currency symbol, but symbols can be ambiguous. Please add a method that formats money using its unambiguous currency code instead.

See wikipedia's section on formatting rules: https://en.wikipedia.org/wiki/ISO_4217#Position_of_ISO_4217_code_in_amounts

In my opinion, formatting using the ISO code should have been the default result of calling Money.ToString() and formatting using the currency symbol should have been the result of calling Money.ToString("C")

We could add extension factory methods for the decimal type, like 123.45.Euro(), etc.

Not sure if this is desired, because it is on a primitive data type in .NET.

Apart from the English name of a Currency, we also want to have a native name of the Currency.

RegionInfo class in .NET has these names available. See RegionInfo.CurrencyNativeName. Check if these are sufficient.

When serializing/deserializing the Money-class, it throws an exception when it tries to set CurrencySymbol.

at System.Globalization.NumberFormatInfo.set_CurrencySymbol(String value)
at NodaMoney.Money.ConvertToString(String format, IFormatProvider formatProvider)
at NodaMoney.Money.ToString()

There's also a problem with formatting, the currency symbol for USD is appended last, when it's supposed to be first.

var c = new Money(100m, "USD");
Console.WriteLine(c.ToString()); // Generates 100,00 $

Serialized output from JavaScriptSerializer

{ 
  "Amount" : 100,
  "Currency" : { 
      "Code" : "USD",
      "DecimalDigits" : 2,
      "EnglishName" : "United States dollar",
      "MajorUnit" : 1,
      "MinorUnit" : 0.01,
      "Number" : "840",
      "Sign" : "$"
    }
}

Hi! I'm using NodaMoney with custom currencies and exchange rates and was very happy to see that custom currencies can be initialized with a high number of decimals. Recently, however, I've discovered that the constructor of the ExchangeRate rounds the instantiated exchange rate value to 6 decimals, which essentially means that I won't be able to use this object for currency conversion, as we need exchange rates with more than 6 decimals.

The culprit is found in ExchangeRate.cs, line 33: Value = Math.Round(rate, 6); // value is a ratio

I've removed the rounding step in my local checkout, but this results in failing tests that involve ExchangeRates that are instantiated with the double data type (instead of decimal). I understand why that happens and why rounding is necessary for this data type, and therefore I would like to discuss the following:

• Why are exchange rates rounded specifically to 6 decimals? (i.e. why six? That number seems arbitrary)
• Why is there a constructor accepting a double in the first place? Especially in the Money domain, I think it would be logical to force the use of decimals when interacting with the library. I would vouch for removing this constructor.
• If removing that constructor is not an option, then an alternative solution would be to add an additional parameter to that constructor indicating to which number of decimals the exchange rate should be rounded. Rounding it down to 6 decimals could be the default value; but hardcoding that value into the code is very nontransparent.

I'm very interested in hearing your opinions on this issue!

A IBAN number is used for Money and has a lot of logic to it. Maybe it's interesting to add an IBAN class to this library? Or is it to much out of scope for this library?

See http://en.wikipedia.org/wiki/International_Bank_Account_Number

Some issue with serializing defaults in this PR.

Nice Newtonsoft dependency is removed. But worth mentioning if just doing a drop-in update, this is a breaking change for users who rely on JsonSerializerSetting to fine tune JSON output. For example CamelCasePropertyContractResolver. It'll no longer have effect on Amount and Currency properties.

[Fact]
public void DefaultMoney_should_roundtrip()
{
    var expected = new MyClass { Value = default };
    // var expected = new MyClass { Value = new Money(0, "USD") }; // This works

    var serializer = new DataContractSerializer(typeof(MyClass));
    using (var ms = new MemoryStream()) {
        serializer.WriteObject(ms, expected);
        ms.Position = 0;

        var clone = serializer.ReadObject(ms); // throws Currency code can't be null error
        expected.Value.Should().Be(0);
    }
}

[DataContract]
private class MyClass
{
    [DataMember]
    public Money Value { get; set; }
}

Initializing Money with Double (or Single) should use the string representation instead of casting, like in JodaMoney. This would lead to the the most expected answer for most programming scenarios. Any Double in code will be converted to exactly the same Money with the same scale. For example, the Double '1.45d' will be converted to '1.45'.

double do = 1.45;
string s = 1.45.ToString(); // or String.Parse()?
decimal de = Decimal.Parse(s);

See also http://stackoverflow.com/a/7186298/8820 (BigDecimal in Java)

Add the English name, the native name and if available the sign of the minor unit.

For examle: United Arab Emirates dirham (AED) has as minor unit (1/100) the English name fils and the native name فلس.

I guess Currency constructor should allow null for "number" parameter, or should use default value (eg. "none").

I have been using the library and have found that I get errors randomly on first use.

I believe I have tracked this down to a race condition in the CurrencyRegistry when constructing the list of currencies. If the currency is not returned it uses the CurrentCulture's currency, which in my case is AUD, but usually using USD in this app.

If you run all tests you'll find that random parse tests fail, this is a symptom of the race problem.

Example test output:

NodaMoney.Tests.MoneyParsableSpec.GivenIWantToTryParseImplicitCurrency.WhenParsingDollarSymbolInArgentina_ThenThisShouldReturnArgentinePeso
   Source: MoneyParsableSpec.cs line 363
   Duration: 3 ms

  Message: 
    NodaMoney.InvalidCurrencyException : The requested operation expected the currency AUD, but the actual value was the currency ARS!
  Stack Trace: 
    Money.AssertIsSameCurrency(Money left, Money right) line 297
    Money.CompareTo(Money other) line 122
    ComparableTypeAssertions`1.Be(T expected, String because, Object[] becauseArgs)
    GivenIWantToTryParseImplicitCurrency.WhenParsingDollarSymbolInArgentina_ThenThisShouldReturnArgentinePeso() line 368

For more details see: http://www.currency-iso.org/en/shared/amendments/iso-4217-amendment.html

At the moment it is not sure if storing Money to MongoDB, RavenDB & Azure DocumentDB works. The assumption is that it works, but it isn't verified.

From 1 July 2016 to 31 December 2016, the banknotes of the 2000 series and the banknotes and coins of the 2009 series will be in parallel circulation and subject to obligatory acceptance without restriction in all kinds of payments to the ratio of 10,000 old for one new ruble (10,000:1).

From 1 January 2017 through 31 December 2021, the currency units of the 2000 series will be exchanged for the currency units of the 2009 series in any sum without restriction and charging commission.

See http://www.currency-iso.org/en/shared/amendments/iso-4217-amendment.html

I wonder if it would be useful to add TaxedMoney type with a distinction between net amount and gross amount, just a proposition though. Tell me what you think.

But then, the following requirements should be met:

• Changing tax updates the amounts, should it calculate from gross or net?
• Two static instantiation methods to be able to instantiate from gross or from net FromNet/FromGross
• Change of gross amount changes net amount, and change of net amount changes gross amount respectively to keep amounts synchronized

Taxation is pretty complex, and it would be great if NodaMoney handled that as well

For more details see: https://www.currency-iso.org/en/shared/amendments/iso-4217-amendment.html.

Nuget package deploys Radme and License files into the root directory of the project. This is a bit unusually. Usual practice is to ask for licence acceptance and to open readme upon installation, but location of readme points to packages/package.
Can something be done regarding this?

See https://help.github.com/articles/syncing-a-fork/

Add all the historic currencies codes and make sure they are marked with IsObsolete = true.

Best way, I think, is to have a 'ISO-421 HISTORIC' namespace.

I think the following should succeed, sort of like https://en.wikipedia.org/wiki/Identity_element

default(Money) + Money.Euro(100) should give Money.Euro(100)

But it throws an exception because the 'currency' of default(Money) is null.

@remyvd and others, what're your opinions?

The class ExchangeRate makes it possible to convert money to different currencies. But you have to manually create these exchange rates to use them.

It would be nice to have a ExchangeRateProvider that can be implemented by users of the library to attach it to their favourite provider(s) of exchange rates.

Java money has some implementations that could be used as a start example

Allow to parse money with suffixes such as €126.36M, or -$2.45B. Four suffixes should be recognized: K, M, B, and T. They can be given in lower or upper case, but must directly follow the number to be interpreted as a multiplier suffix.

In this case K is thousand. The numeric value of Billion is culture dependent.

See the comments of #8 for the start issue.

ExchangeRate is/can't be dated at the moment. It would be nice to add the dates where the ExchangeRate is valid between.

Possible implementations could be:

// override of constructor
var fx = new ExchangeRate("EUR", "USD", 1.14m, dateFrom, dateTo)
var fx = new ExchangeRate("EUR", "USD", 1.14m, new Range<DateTime>(dateFrom, dateTo))
var fx = new ExchangeRate("EUR", "USD", 1.14m, new Period(dateFrom, dateTo))

// or be external
var fx = new ExchangeRate("EUR", "USD", 1.14m)
var datedFx = new Range<DateTime>(dateFrom, dateTo, fx);
var datedFx = new Period(dateFrom, dateTo, fx);

This may or may not qualify as an issue for the library as a whole.

The 0.5.0.0 release of NodaMoney on NuGet adds a project dependency upon System.Collections.Concurrent, for the purpose of two ConcurrentDictionaries in the CurrencyRegistry. System.Collections.Concurrent does not appear to be broadly compatible with all PCL profiles, or at least the one that we're dependent upon (78), and so we're unable to use this version. Given the apparently very limited application of System.Collections.Concurrent classes, I wanted to at least float the possibility of removing the dependency in future releases (would locks suffice?).

I know I probably should just utilize a private fork and address this issue on my own, but my organization won't be supporting VS2015/C#6 for a while yet, which means needing to base off months old code before the conversion, and just generally putting up with a whole bunch of maintenance and maintainability headaches. :)

Thanks for reading; great library! I dearly want to make use of it. :)

Hi,
Since Noda's Money is a struct type, we cannot use it as field type for our entities in our .Net Core 2.0 project, using Entity Framework Core 2.0.

Since EF Core 2.0 now fully supports owned entities, it would be great to be able to do that!

Could you advise us a way to do that?
Thanks

We could add the possible coins and banknotes to Currency. For example for Euro (EUR) we have the following:

• Coins:
  • Freq Used: €1, €2, 5cent, 10cent, 20cent, 50cent
  • Rarely Used: 1cent, 2cent
• Banknotes:
  • Freq Used: €5, €10, €20, €50, €100
  • Rarely Used: €200, €500

Is this useful for anyone and how would they want to use it?

What is the source of the data in InitializeIsoCurrencies? Is it taken from Wikipedia?(https://github.com/remyvd/NodaMoney/blob/master/src/NodaMoney/CurrencyRegistry.cs#L135)

I'm asking because I use files from currency-iso.org in my application. They seem to be more up-to-date and complete than what NodaMoney provides: http://www.currency-iso.org/en/home/tables/table-a1.html

Version: 1.0.5

It seems that, with some currencies, an error is thrown as ToString is trying to use the DecimalDigits value of the currency, which isn't in the valid range of 0 to 99, inclusive.

Example

new NodaMoney.Money(636, "XAG").ToString()

The XAG currency seems to have a DecimalDigits value of NotApplicable

https://github.com/remyvd/NodaMoney/blob/4d3372c44746f65c0886fec142fd65d5fc4890ef/src/NodaMoney/CurrencyRegistry.cs#L271

which resolves to -1 as an integer, so when ToString is called, the following line in Money.GetFormatProvider throws an System.ArgumentOutOfRangeException

https://github.com/remyvd/NodaMoney/blob/8e0a22c110e8620d35a1374c4e6c8483689226fc/src/NodaMoney/Money.Formattable.cs#L61

Serialization of the Money object failes if embedded in another object.

For example I have to following class

    [DataContract]
    public class Article
    {
        [DataMember]
        public int Id { get; set; }
        [DataMember]
        public string Name { get; set; }
        [DataMember]
        public Money Amount { get; set; }
    }

and the following object

var article = new Article
{
       Id = 123,
       Amount = Money.Euro(27.15),
       Name = "Foo"
};

will result in that xml string:

<Article xmlns="...">
    <Amount Amount="27.15" Currency="EUR"/>
    <Id>123</Id>
    <Name>Foo</Name>
</Article>

When reading that string NodaMoney complaints about the LocalName differing from "Money". It's Amount in my example...

We should be able to convert a string representation of money to its Money equivalent using the specified style and culture-specific format. A return value indicates whether the conversion succeeded or failed.

See also http://msdn.microsoft.com/en-us/library/system.decimal.parse%28v=vs.110%29.aspx and http://msdn.microsoft.com/en-us/library/ew0seb73%28v=vs.110%29.aspx

When running unit tests a Specs.txt file is generated. Very usefull, by the way! But I don't think it should be added to version control.

According to https://en.wikipedia.org/wiki/ISO_4217 CSD (Serbian Dinar) is valid currency code. However, when attempting to instantiate Money object with that currency code, exception is thrown

CSD is an unknown ISO-4217 currency code!

This is a test issue to see if its synced correctly to Trello.

By adding namespace functionality we can support legacy and custom currencies (like BitCoin), besides the ISO currencies. See also the newly implemented CurrencyUnit in Java 8/9.

The default should be ISO, but something like an override would be useful:

var eur = Currency.FromCode("EUR");
var xbt = Currency.FromCode("XBT", "Custom"); // or an enum

Also think how developers can add there own currencies by registering them. See an example for Cultures in .NET: http://msdn.microsoft.com/en-us/library/ms172469%28v=VS.100%29.aspx.

How should we name the classes? CurrencyFormatInfo(Builder), MoneyFormatInfo(Builder)

Some countries are allowed to not use all the available coins within a currency when dealing with cash. For example in some European countries, like the Netherlands.

See http://www.dnb.nl/en/payments/euro-banknotes-and-coins/rounding-to-multiples-of-5-cents/ and http://en.wikipedia.org/wiki/Swedish_rounding.

See http://www.w3schools.com/charsets/ref_utf_currency.asp.

Is this useful, because there aren't many of them?

Would anyone consider including ExchangeRate JsonConverter?

Please create and publish packages for the .NET Platform Standard (netstandard1.0).

Nuget package updater throws error when trying to update package from 0.5.0 to 0.6.0 .

Could not install package 'NodaMoney.Serialization.AspNet 0.6.0'. You are trying to install this package into a project that targets '.NETFramework,Version=v4.5.2', but the package does not contain any assembly references or content files that are compatible with that framework. For more information, contact the package author.

These should be updated. Some examples:

• Turkish lira (TRY) is now YTL, but is really ₺
• United Arab Emirates dirham (AED) is now ¤, but is really د.إ
• Philippine peso (PHP) is now P, but is really ₱

Can we use the build-in Region-info? Has a lot of info, but is it 100% accurate. Also depends on fonts.

For example:
var ci = new CultureInfo("nl-NL");
var currency = ci.GetCurrency();

This issue should also take into consideration the fact that a certain Culture might have 2 currencies.
One might be the official Currency, the other Currency might the most used Currency. For example many countries use the dollar for trading and/or paying for commodities, this instead of their official Currency.

For more details see: http://www.currency-iso.org/en/shared/amendments/iso-4217-amendment.html

default(Money) fails serialization round-trip of MoneyJsonConverter. Should that be fixed?

[Fact]
public void Money_default_should_serialize()
{
    var expected = new Money();
    var json = JsonConvert.SerializeObject(expected);
    var actual = JsonConvert.DeserializeObject<Money>(json); // Fails here
    actual.Should().NotBeNull();
    actual.Currency.Should().Be(expected.Currency);
    actual.Amount.Should().Be(expected.Amount);
}

New methods Parse and TryParse are added to Money, but not explained in the readme. This should be added, especially how the implicit parsing works.