Skip to content

Commit

Permalink
Refactor money formatting logic and improve performance.
Browse files Browse the repository at this point in the history 
Consolidated formatting logic with cleaner code using switch expressions, reducing verbosity and improving maintainability. Updated performance benchmarks show significant enhancements in arithmetic and parsing operations. Simplified Money increment/decrement operations to eliminate unnecessary object allocations.
  • Loading branch information
@RemyDuijkeren
RemyDuijkeren committed Jan 25, 2025
1 parent 0684a06 commit cc67bcf
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 92 deletions.
129 changes: 56 additions & 73 deletions src/NodaMoney/CurrencyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,11 +319,18 @@ public static CurrencyInfo GetInstance(IFormatProvider? formatProvider)
/// <inheritdoc />
public string Format(string? format, object? arg, IFormatProvider? formatProvider)
// => Format(format.AsSpan(), arg, formatProvider);
// public string Format(ReadOnlySpan<char> format, object? arg, IFormatProvider? formatProvider)
//private string Format(ReadOnlySpan<char> format, object? arg, IFormatProvider? formatProvider)
{
// TODO: Add Round-trip format specifier (R) https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-numeric-format-strings#round-trip-format-specifier-r
// TODO: ICustomFormat : http://msdn.microsoft.com/query/dev12.query?appId=Dev12IDEF1&l=EN-US&k=k(System.IFormatProvider);k(TargetFrameworkMoniker-.NETPortable,Version%3Dv4.6);k(DevLang-csharp)&rd=true
// TODO: Hacked solution, solve with better implementation
// Supported formats: see https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-numeric-format-strings
// G: General format = C but with currency code => ISO code with number, like EUR 23.002,43 , EUR 23,002.43, 23,002.43 EUR
// C: Currency Symbol format, like € 23.002,43 , € 23,002.43, 23,002.43 €
// C => TODO: if symbol is GenericCurrencySign, then use code? What if NoCurrency?
// C => TODO: use C for long version (US$) and c for short version ($) in some locals
// R: Round-trip format with currency code
// N: Number format = decimal
// F: Fixed point format = decimal
// L: English name, like 23.002,43 dollar
// l: Native name, like 23.002,43 dólar

if (arg is null)
throw new ArgumentNullException(nameof(arg));
Expand All @@ -336,19 +343,7 @@ public string Format(string? format, object? arg, IFormatProvider? formatProvide
: arg.ToString() ?? string.Empty;
}

// Supported formats: see https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-numeric-format-strings
// G: General format = C but with currency code => ISO code with number, like EUR 23.002,43 , EUR 23,002.43, 23,002.43 EUR
// C: Currency Symbol format, like € 23.002,43 , € 23,002.43, 23,002.43 €
// C => TODO: if symbol is GenericCurrencySign, then use code? What if NoCurrency?
// C => TODO: use C for long version (US$) and c for short version ($) in some locals
// R: Round-trip format with currency code
// N: Number format = decimal
// F: Fixed point format = decimal
// L: English name, like 23.002,43 dollar
// l: Native name, like 23.002,43 dólar

// TODO: short= $13B, $12.8B or long= $14 billion

// TODO: CLDR-data: https://github.com/unicode-org/cldr-json/tree/main/cldr-json/cldr-numbers-full
// For example USD in NL
// "USD": {
Expand All @@ -371,25 +366,29 @@ public string Format(string? format, object? arg, IFormatProvider? formatProvide
return money.Amount.ToString(format, nfi);
}

switch (fmt)
return fmt switch
{
case 'C' when digits == -1:
return money.Amount.ToString("C", nfi);
case 'C' or 'c': // TODO: use C for long version (US$) and c for short version ($) in some locals?
return money.Amount.ToString(format, nfi);
case 'G' or 'g':
nfi = ToNumberFormatInfo(formatProvider, true); // replace currency symbol with code
return money.Amount.ToString($"C{format.AsSpan(1).ToString()}", nfi);
case 'L': // 1 US dollar, 0 US dollars, 1.000 US dollars, 1,000 US dollars
case 'l': // future: use lower-case for local native name
// Currency formats
'C' when digits == -1 => money.Amount.ToString("C", nfi),
'C' or 'c' => // TODO: use C for long version (US$) and c for short version ($) in some locals?
money.Amount.ToString($"C{digits}", nfi),

// General format (uses currency code as symbol)
'G' or 'g' when digits == -1 => money.Amount.ToString("C", ToNumberFormatInfo(formatProvider, true)),
'G' or 'g' => money.Amount.ToString($"C{digits}", ToNumberFormatInfo(formatProvider, true)),

// English Name currency (e.g., "1234.56 US dollars") // TODO: future use lower-case for local native name
'L' or 'l' when digits == -1 =>
// N will use NumberDecimalDigits instead of CurrencyDecimalDigits.
nfi.NumberDecimalDigits = nfi.CurrencyDecimalDigits;
return $"{money.Amount.ToString($"N{format.AsSpan(1).ToString()}", nfi)} {EnglishName}";
case 'R' or 'r':
return $"{Code} {money.Amount.ToString(format, nfi)}";
default:
return money.Amount.ToString(format, nfi);
}
$"{money.Amount.ToString($"N{nfi.CurrencyDecimalDigits}", nfi)} {EnglishName}",
'L' or 'l' => $"{money.Amount.ToString($"N{digits}", nfi)} {EnglishName}",

// Round-trip format (e.g., "USD 1234.56")
'R' or 'r' when digits == -1 => $"{Code} {money.Amount.ToString("R", nfi)}",
'R' or 'r' => $"{Code} {money.Amount.ToString($"R{digits} ", nfi)}",

_ => money.Amount.ToString(format, nfi)
};
}

/// <summary>
Expand All @@ -401,16 +400,12 @@ public string Format(string? format, object? arg, IFormatProvider? formatProvide
/// <returns>A <see cref="NumberFormatInfo"/> instance configured with currency formatting properties specific to the current currency.</returns>
private NumberFormatInfo ToNumberFormatInfo(IFormatProvider? formatProvider, bool useCurrencyCode = false)
{
NumberFormatInfo numberFormatInfo = (NumberFormatInfo)CultureInfo.CurrentCulture.NumberFormat.Clone();
if (formatProvider != null)
NumberFormatInfo numberFormatInfo = formatProvider switch
{
numberFormatInfo = formatProvider switch
{
CultureInfo ci => (NumberFormatInfo)ci.NumberFormat.Clone(),
NumberFormatInfo nfi => (NumberFormatInfo)nfi.Clone(),
_ => numberFormatInfo // use current culture
};
}
CultureInfo ci => (NumberFormatInfo)ci.NumberFormat.Clone(),
NumberFormatInfo nfi => (NumberFormatInfo)nfi.Clone(),
_ => (NumberFormatInfo)CultureInfo.CurrentCulture.NumberFormat.Clone()
};

numberFormatInfo.CurrencyDecimalDigits = DecimalDigits;
numberFormatInfo.CurrencySymbol = Symbol;
Expand All @@ -421,39 +416,27 @@ private NumberFormatInfo ToNumberFormatInfo(IFormatProvider? formatProvider, boo
// Replace currency symbol with the code
numberFormatInfo.CurrencySymbol = Code;

// For PositivePattern and NegativePattern add space between code and value
if (numberFormatInfo.CurrencyPositivePattern == 0) // $n
numberFormatInfo.CurrencyPositivePattern = 2; // $ n
if (numberFormatInfo.CurrencyPositivePattern == 1) // n$
numberFormatInfo.CurrencyPositivePattern = 3; // n $
// For PositivePattern add space between code and value
numberFormatInfo.CurrencyPositivePattern = numberFormatInfo.CurrencyPositivePattern switch
{
0 => 2, // $n -> $ n
1 => 3, // n$ -> n $
_ => numberFormatInfo.CurrencyPositivePattern // No change needed
};

switch (numberFormatInfo.CurrencyNegativePattern)
// For NegativePattern add space between code and value
numberFormatInfo.CurrencyNegativePattern = numberFormatInfo.CurrencyNegativePattern switch
{
case 0: // ($n)
numberFormatInfo.CurrencyNegativePattern = 14; // ($ n)
break;
case 1: // -$n
numberFormatInfo.CurrencyNegativePattern = 9; // -$ n
break;
case 2: // $-n
numberFormatInfo.CurrencyNegativePattern = 12; // $ -n
break;
case 3: // $n-
numberFormatInfo.CurrencyNegativePattern = 11; // $ n-
break;
case 4: // (n$)
numberFormatInfo.CurrencyNegativePattern = 15; // (n $)
break;
case 5: // -n$
numberFormatInfo.CurrencyNegativePattern = 8; // -n $
break;
case 6: // n-$
numberFormatInfo.CurrencyNegativePattern = 13; // n- $
break;
case 7: // n$-
numberFormatInfo.CurrencyNegativePattern = 10; // n $-
break;
}
0 => 14, // ($n) -> ($ n)
1 => 9, // -$n -> -$ n
2 => 12, // $-n -> $ -n
3 => 11, // $n- -> $ n-
4 => 15, // (n$) -> (n $)
5 => 8, // -n$ -> -n $
6 => 13, // n-$ -> n- $
7 => 10, // n$- -> n $-
_ => numberFormatInfo.CurrencyNegativePattern // No change needed
};

return numberFormatInfo;
}
Expand Down
4 changes: 2 additions & 2 deletions src/NodaMoney/Money.UnaryOperators.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ public partial struct Money
/// <summary>Increments the specified money.</summary>
/// <param name="money">The money.</param>
/// <returns>The result.</returns>
public static Money Increment(in Money money) => Add(money, new Money(money.Currency.MinimalAmount, money.Currency));
public static Money Increment(in Money money) => Add(money, money.Currency.MinimalAmount);

/// <summary>Decrements the specified money.</summary>
/// <param name="money">The money.</param>
/// <returns>The result.</returns>
public static Money Decrement(in Money money) => Subtract(money, new Money(money.Currency.MinimalAmount, money.Currency));
public static Money Decrement(in Money money) => Subtract(money, money.Currency.MinimalAmount);
}
30 changes: 15 additions & 15 deletions tests/Benchmark/PerformanceReport.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ AMD Ryzen 7 5800H with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
| Increment | 374.80 ns | 7.507 ns | 11.907 ns | - |
| Decrement | 369.70 ns | 7.295 ns | 14.229 ns | - |
#### after
| Method | Mean | Error | StdDev | Gen0 | Allocated |
|--------------------------|-----------:|----------:|----------:|-------:|----------:|
| Addition | 12.311 ns | 0.2696 ns | 0.3599 ns | - | - |
| Subtraction | 17.913 ns | 0.3833 ns | 0.5497 ns | - | - |
| CompareSameCurrency | 3.570 ns | 0.0905 ns | 0.0846 ns | - | - |
| CompareDifferentCurrency | 3.833 ns | 0.1055 ns | 0.1579 ns | - | - |
| CompareAmount | 4.470 ns | 0.0475 ns | 0.0421 ns | - | - |
| Increment | 105.702 ns | 1.3173 ns | 1.1000 ns | 0.0038 | 32 B |
| Decrement | 110.203 ns | 0.8956 ns | 0.7939 ns | 0.0038 | 32 B |
| Method | Mean | Error | StdDev | Gen0 | Allocated |
|--------------------------|----------:|----------:|----------:|-------:|----------:|
| Addition | 16.804 ns | 0.1825 ns | 0.1618 ns | - | - |
| Subtraction | 15.575 ns | 0.1926 ns | 0.1707 ns | - | - |
| CompareSameCurrency | 3.639 ns | 0.0401 ns | 0.0356 ns | - | - |
| CompareDifferentCurrency | 3.667 ns | 0.0303 ns | 0.0253 ns | - | - |
| CompareAmount | 3.897 ns | 0.0780 ns | 0.0609 ns | - | - |
| Increment | 98.307 ns | 0.7919 ns | 0.7020 ns | 0.0038 | 32 B |
| Decrement | 99.775 ns | 1.5502 ns | 1.3743 ns | 0.0038 | 32 B |

## MoneyFormatting
#### before (v1.x)
Expand All @@ -70,12 +70,12 @@ AMD Ryzen 7 5800H with Radeon Graphics, 1 CPU, 16 logical and 8 physical cores
| Explicit | 271.5 ns | 5.50 ns | 9.33 ns | 0.0525 | 440 B |
| ExplicitWithFormat | 270.3 ns | 5.42 ns | 12.56 ns | 0.0525 | 440 B |
#### after
| Method | Mean | Error | StdDev | Median | Gen0 | Allocated |
|--------------------|---------:|--------:|---------:|---------:|-------:|----------:|
| Implicit | 107.3 ns | 2.19 ns | 3.95 ns | 107.7 ns | 0.0468 | 392 B |
| ImplicitWithFormat | 113.3 ns | 2.27 ns | 3.25 ns | 112.8 ns | 0.0468 | 392 B |
| Explicit | 143.9 ns | 2.89 ns | 5.84 ns | 144.3 ns | 0.0842 | 704 B |
| ExplicitWithFormat | 154.6 ns | 3.97 ns | 11.65 ns | 151.1 ns | 0.0842 | 704 B |
| Method | Mean | Error | StdDev | Gen0 | Allocated |
|--------------------|---------:|--------:|--------:|-------:|----------:|
| Implicit | 109.6 ns | 2.24 ns | 4.14 ns | 0.0467 | 392 B |
| ImplicitWithFormat | 143.4 ns | 2.89 ns | 3.33 ns | 0.0505 | 424 B |
| Explicit | 111.5 ns | 2.25 ns | 4.44 ns | 0.0468 | 392 B |
| ExplicitWithFormat | 140.2 ns | 2.75 ns | 5.43 ns | 0.0505 | 424 B |

## MoneyParsing
#### before (v1.x)
Expand Down
4 changes: 2 additions & 2 deletions tests/Benchmark/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

// var initializingCurrencyReport = BenchmarkRunner.Run<InitializingCurrencyBenchmarks>();
// var initializingMoneyReport = BenchmarkRunner.Run<InitializingMoneyBenchmarks>();
// var moneyOperationsReport = BenchmarkRunner.Run<MoneyOperationsBenchmarks>();
var moneyFormattingReport = BenchmarkRunner.Run<MoneyFormattingBenchmarks>();
var moneyOperationsReport = BenchmarkRunner.Run<MoneyOperationsBenchmarks>();
// var moneyFormattingReport = BenchmarkRunner.Run<MoneyFormattingBenchmarks>();
// var moneyParsingReport = BenchmarkRunner.Run<MoneyParsingBenchmarks>();
// var addingCustomCurrencyReport = BenchmarkRunner.Run<AddingCustomCurrencyBenchmarks>();
// var highLoadBenchReport = BenchmarkRunner.Run<HighLoadBenchmarks>();
Expand Down

0 comments on commit cc67bcf

Please sign in to comment.