3

Please see the code below:

public class EnglishCurrency : Currency
{
    private static readonly decimal[] _denominations= 
    {
         0.01M,  0.02M,  0.05M, 
         0.10M,  0.20M,  0.50M,
         1.00M,  2.00M,  5.00M,
        10.00M, 20.00M, 50.00M,
    };

    public override IEnumerable<decimal> GetDenominations() => _denominations;
}

Is it a bad idea to have a class that contains constants only like this? The reason I am doing this is so that I can inject a Currency (EnglishCurrency; SpanishCurrency etc) into another class. However, I wander if there is a better way of approaching it.

Everywhere I am reading suggests that a class should not contain constants only. Therefore I believe there may be another approach.

  • 1
    Why the public override? In fact, why is it even a method? Just do public decimal[] Values {get} = { 0.1M, ... }; – David Arno Dec 14 '17 at 15:38
  • 1
    "Everywhere I am reading suggests that a class should not contain constants only". Then I'd suggest you are reading in the wrong places. That's the price you pay though for heading down the RDM rabbit hole... :) – David Arno Dec 14 '17 at 15:40
  • @DavidArno polymorphism – TheCatWhisperer Dec 14 '17 at 15:50
  • 2
    @DavidArno Sure that's a probably a better choice but you agree with the larger point, I gather. – JimmyJames Dec 14 '17 at 15:58
  • 2
    "However, a EnglishCurrency is a Currency. Therefore I am thinking it should be inheritance" I think you are putting too much into the difference between interfaces and base classes. The is-a relationship also applies to interfaces. – JimmyJames Dec 14 '17 at 17:01
5

This is fine. Here's the thing, your class does not contain only constants!

It has the method GetDenominations().

I do not see anything wrong with this class at all, keep up the good work :)

|improve this answer|||||
  • Thanks. Would this be a value type in the DDD world? Also should it have a .Equals and .Hashcode? – w0051977 Dec 14 '17 at 15:22
  • @w0051977 Why did you edit the question? I liked the first class... this class makes no sense – TheCatWhisperer Dec 14 '17 at 15:23
  • 5
    I agree. There is far too much programming/design advice that focuses on how to write code with no context around problem solving. If your solution works, is easy to understand and can be easily replaced when you need to add to the design, it's fine. All of these seem to be met by this approach. – JimmyJames Dec 14 '17 at 15:23
  • @TheCatWhisperer , I was editing my question as you were answering to try and make it clearer. However, I think you have understood it as it was so I have changed it back. Could you answer the question in my first comment? Thanks. – w0051977 Dec 14 '17 at 15:27
  • 1
    I would note that the EnglishSummertime example seems to be kind of silly, though. You could have a single summertime class that takes the months as a parameter. It still is easy to swap out with a real implementation so not a problem per se. Sometimes it's valuable to do something like this when you are working on another part of the application and want to defer some work for later. – JimmyJames Dec 14 '17 at 15:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.