Value Objects member used to validate another value object

Question

Please see the code below:

 public sealed class UKCurrency : ICurrency
 {
 private static readonly int _decimalPlaces=2; 
 private static readonly decimal[] _denominations = new decimal[] {
 50.00M, 20.00M, 10.00M,
 5.00M, 2.00M, 1.00M,
 0.50M, 0.20M, 0.10M,
 0.05M, 0.02M, 0.01M,
 };
 public IEnumerable<decimal> Denominations
 {
 get { foreach (var denomination in _denominations) yield return denomination; }
 }
 }
 public sealed class DenominationCounter
 {
 private readonly decimal _cost;
 public decimal Cost
 {
 get { return _cost; }
 }
 public ICurrency Currency
 {
 get { return _currency; }
 }
 public DenominationCounter(decimal cost, ICurrency currency)
 {
 if (currency == null)
 throw new ArgumentNullException("Currency cannot be null", "ICurrency");
 if (cost < 0)
 throw new ArgumentException("Cost cannot be negative", "Cost");
 if (decimal.Round(cost, currency.DecimalPlaces) != cost)
 throw new ArgumentException(string.Concat("Cost has too many decimal places. It should only have: ", currency.DecimalPlaces), "Cost");
 _cost = cost;
 _currency = currency;
 }
public IEnumerable<System.Collections.Generic.KeyValuePair<decimal, int>> CalculateDenominations()
 {
 var target = _cost;
 foreach (var denomination in _currency.AvailableDenominations)
 {
 var numberRequired = target / denomination;
 if (numberRequired >= 1)
 {
 int quantity = (int)Math.Floor(numberRequired);
 yield return new KeyValuePair<decimal, int>(denomination, quantity);
 target = target - (quantity * denomination);
 }
 }
 }
 }

The DenominationCounter constructor throws an exception if the cost has the wrong number of decimal places.

Notice that the UKCurrency class is used to validate the DenominationCounter as shown below:

if (decimal.Round(cost, currency.DecimalPlaces) != cost)

Is this a normal to approach validation like this:

1) A Value Objects member is used to validate an entity

2) A Value Objects member is used to validate another value object

I am asking this because I have never seen validation approached like this before and I am trying to follow the principle of least astonishment these days.

• Ewan: I'd still put the same answer. not sure I understand your question
• Ewan: for a start neither of your objects is a struct
• w0051977: @Ewan, I am using objects because of this: enterprisecraftsmanship.com/2017/12/04/…
• w0051977: @Ewan, I am asking if it is normal for the Currency class to be used in the validation of the DenominationCounter class i.e. is it normal to do: if (decimal.Round(cost, currency.DecimalPlaces) != cost)
• Ewan: last time I was immediately downvoted twice. so im obviously missing something that at least 2 other people get

Answer 1

I don't see any need for your DenominationCounter object. It's purpose is to provide an IEnumerable<(decimal, int)> (I've simplified your data type to use a tuple) via CalculateDenominations.

But that method is completely deterministic in nature. You are achieving nothing useful (beyond arguments around primitive obsession) by having instances.

Further, you are creating a non-cohesive solution to denominations by having multiple classes responsible for handling what should be a single responsibility. Don't just use subtype polymorphism just for the sake of it.

I'd make the whole lot a single static class, to maximise cohesion, to remove all the complications around constructors, to remove the need to duplicate functionality in every implementation of ICurrency.Denomination etc:

public enum Currency { GBP, EUR, USD, ... }
public static class Denominations
{
 private static readonly decimal[] GBPDenominations = new decimal[]
 {
 50.00M, 20.00M, 10.00M,
 5.00M, 2.00M, 1.00M,
 0.50M, 0.20M, 0.10M,
 0.05M, 0.02M, 0.01M,
 };
 private static readonly decimal[] EURDenominations = ...
 private static Dictionary<Currency, decimal[]> CurrencyDenominations =
 new Dictionary<Currency, decimal[]>
 {
 [Currency.GBP] = GBPDenominations,
 [Currency.EUR] = EURDenominations,
 ...
 };
 public static IEnumerable<(decimal denomination, int quantity)>
 CalculateDenominations(Currency currency, decimal cost)
 {
 var remainingCost = cost;
 foreach (var denomination in CurrencyDenominations[currency])
 {
 var numberRequired = remainingCost / denomination;
 if (numberRequired >= 1)
 {
 var quantity = (int)Math.Floor(numberRequired);
 yield return (denomination, quantity);
 remainingCost = remainingCost - (quantity * denomination);
 }
 }
 }
}

• w0051977: Could you explsin what you mean by: "But that method is completely deterministic in nature.". Is your code a Domain Service? I thought domain services were none deterministic?
• David Arno: I honestly do not know what a domain service is. CalculateDenominations only uses constant values and its parameters to produce a result. It has no side effects (it doesn't access or modify external state). Therefore every time you call eg CalculateDenominations(Currency.EUR, 34.77M), you will get the same response, for the lifetime of the application. Therefore CalculateDenominations is deterministic.
• w0051977: last question - when you say "it doesn't access or modify external state" I assume you mean instance variables? Thanks.
• David Arno: Not necessarily. External state could be instance or static variables, both within the class and in other classes. But it also covers other methods that access such state, as well as things external to the application (file systems, environment state, eg OS type, databases and so forth).
• w0051977: Can I ask where the validation is?

Answer 2

You don't have two different value objects. You have a value object and a fancy array. You probably need to combine a lot of UKCurrency and DenominationCounter. It's not wrong for a value object to validate that its dependencies are valid even if they are other value objects, but both objects need to be valuable independent of each other.

A value object needs a value that's actually comparable (even if you never do or never implement a comparison), UKCurrency doesn't have that. Every instance of UKCurrency is exactly the same, and even worse if CanadianCurreny existed it would be functionally identical to UKCurrency.

Try stating what your objects do/ what value they provide in 1-2 sentences. If you can't do that or what you said doesn't seem useful then something needs to change.

• w0051977: would you describe the code in my question as a domain service?

Answer 3

OK So! here is my understanding of your approach

1. In your domain you have a special value type eg. Currency In Denominations
2. You want to prevent the construction of a value type that would be impossible by definition, ie half a 10p piece.
3. You are modeling value types as immutable reference types. rather than using value types. Because of reasons.
4. Some of the information required to validate the 'value type' is contained in another class. eg the possible denominations of a particular currency.

Bad things:

1. Throwing in constructors is generally bad. The exception to the rule is impossible value types eg 31st of Feb
2. All of your examples could be refactored to do the validation in a method rather than a constructor.

If you are married to your way of doing things, then you have no alternative to your current solution. Yes it seems slightly odd, but it is functional.

If you want a solution where you don't have that particular combination of validation, why not get rid of the constructor exceptions completely?

public class ImmutableListOfCoins 
{
 private IEnumerable<KeyValuePair<decimal, uint>> listOfCoins {get; private set;}
 public ImmutableListOfCoins(IEnumerable<KeyValuePair<decimal, uint>> listOfCoins) 
 {
 ....
 }
}
public static ImmutableListOfCoins CalculateDenominations(decimal cost)
 {
 var target = _cost;
 if (decimal.Round(cost, currency.DecimalPlaces) != cost)
 {
 throw new ArgumentException("Cost has too many decimal places.");
 }
 var loc = new Dictionary<decimal,uint>();
 foreach (var denomination in _currency.AvailableDenominations)
 {
 var numberRequired = target / denomination;
 if (numberRequired >= 1)
 {
 var quantity = (uint)Math.Floor(numberRequired);
 loc.Add(new KeyValuePair<decimal, uint>(denomination, quantity));
 target = target - (quantity * denomination);
 }
 }
 return new ImmutableListOfCoins(loc);
 }

• w0051977: Would this class exist in the core of the application. From what I can see it is neither an entity nor a value object?
• Ewan: no, list of coins is your value object. you have a service or factory class to create them from other inputs which may be invalid
• w0051977: So the list of coins value object would be contained in the core. Where would your CalculateDenominations object be contained - in a domain service in the core?
• Ewan: depends on the rest of your model I guess. in your example its pretty hard to think of an alternate bit of logic. but If we go back to the player.fitness you can imagine you might want to create games where the validation critiera change or are different. perhaps for the olympics you can only create teams where the fitness level of players is 10x the min requirement
• Ewan: EwansAmazingCostCalcService