1

My question is similar to this one: Storing data in code

Please see the code below:

public class EnglishCurrency 
    {
        public override System.Collections.Generic.IEnumerable<decimal> Values()
        {
        yield return 0.01M;
        yield return 0.02M;
        yield return 0.05M;
        yield return 0.1M;
        yield return 0.2M;
        yield return 0.5M;
        yield return 1M;
        yield return 2M;
        yield return 5M;
        yield return 10M;
        yield return 20M;
            yield return 50M;           
        }
    }

It does not seem right from a maintainability point of view to store the currency values in the source code. However, currencies don't change often (the £2 coin was introduced in 1998). Like the other question; I have a few options:

1) The above

2) Store values as XML. Is a WPF app best for this?

3) Store values as JSON

4) Another approach?

In the example in my link; the asker talks about storing countries in source code. Countries are likely to change a lot more than currencies so I can understand why the answerer talks about XML files. Does this also apply to me with values that are less likely to change?

  • Why would it be a bad idea? – immibis Nov 29 '17 at 22:39
  • Alternatively you could define them in an array instead of all those yield statements – Ben Cottrell Nov 29 '17 at 22:40
  • 1
    @w0051977 So if I'm reading your argument right, you are thinking "The purpose of a database is to store data, therefore if I want to store data, it must be in a database"? – immibis Nov 29 '17 at 22:49
  • 1
    The purpose of Visual Basic is to write programs, therefore if you want to write programs, they must be in Visual Basic. – immibis Nov 29 '17 at 22:49
  • 2
    @w0051977 having a private static readonly collection for lookup data is fairly normal. If I were reviewing code like this, my main issue with the usage of the yield statement is that it's an unconventional way of writing a constant lookup table (See Principle of Least Astonishment ) - Also, you can have random access with an Array / List / Dictionary / etc. The yield keyword doesn't provide random access. – Ben Cottrell Nov 29 '17 at 22:50
7

These currency denominations are constants - i.e. those stable, never-changing values which nobody expects to change except in rare circumstances, similar to Pi, the gravitational constant, etc.

Of course, some constants do inevitably sometimes change in the real world, but the commercial cost of re-compiling code for these rare occurrences (and distributing a software update) is usually low/insignificant.

It is completely normal for such constants to exist in a domain, therefore also normal to hard-code constants into the program rather than keeping them in an external persistent store / retrieving from a service / etc.

Related to your example - two potential issues with the yield keyword:

  1. Potential violation of the Princple of Least Astonishment
  2. It doesn't support Random Access - which may or may not be an issue, but I see no reason not to allow this

As a possible alternative to yield, you might consider an array - e.g.

public class EnglishCurrency 
{
    private static readonly decimal[] _values = 
    {
         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> Values() => _values;
}

Edit - The above code uses C# 6.0 Syntax for the Values() method. If that doesn't work due to running an older version of Visual Studio or older version of the C# compiler, then use this instead:

    public override IEnumerable<decimal> Values()
    {
        return _values;
    }
  • That is what I thought. I will test this code tomorrow and then come back to give credit. Thanks. – w0051977 Nov 29 '17 at 23:02
  • Why have you decided to use a static variable? It makes testing harder. Thanks. – w0051977 Nov 29 '17 at 23:06
  • 2
    @w0051977 How does it make testing harder? If these are truly constant values, then I am assuming you aren't looking to ever mutate the state of your Values IEnumerable during the lifetime of your program? – Ben Cottrell Nov 29 '17 at 23:06
  • Because of mocking. Can't think of a reason I would want to mock that variable though - it is a constant. Perhaps the statement in my last comment was too universal. – w0051977 Nov 29 '17 at 23:14
  • 1
    @w0051977 I'm afraid I don't understand your point, what exactly would you be mocking in this case? Are you talking about deriving a mock object from EnglishCurrency? (MockCurrency perhaps?) If so, I can't think of any reason why you couldn't let your mock override the Values method if necessary. The static variable wouldn't get in your way of this. – Ben Cottrell Nov 29 '17 at 23:16
2

Everything has a cost, and sometimes the benefits aren't worth what you have to pay for them. All abstractions have a cost, either in runtime speed, development speed, or brain-cell demands. Part of the craft of software development is having a key eye where the benefits are worth it.

Hardcoding things is the easiest upfront means of putting data into a program. It's simple to do, but its cost is the difficulty of changing the code. It's also basically the fastest way you can get at the data, and it is virtually guaranteed to work every time without exception. Only people programming space probes and pacemakers have to worry about it not working.

On the other hand, say you put the data into a file or database. Now you have a relative ton of the additional work of figuring out how to get at that data. If you are using a config file, you now have to handle the file being missing, and dealing with that. What if users want defaults if they don't have a configuration? Where do you store the defaults? Where do you store the data that says where to find the file? The rabbit hole can do deep indeed. All those questions have to be answered. Hardcoding has none of those.

The more abstractions you pile on the more places bugs can lurk, the trickier it is to test, and the harder you have to think to understand what is taking place in your program.

Sure, DB libraries, OEMS, config file libraries can make some of that work easier, but they will all without a doubt be more work, more code, and more places to go wrong than a hardcoded array.

So you have to evaluate what's appropriate. I can't give you an answer you can mechanically test because so far its a judgment call only us humans can do. You need to evaluate how likely the data is to change, how expensive dealing with that change is going to cost, against the cost of dealing with the abstractions. Like many programs, you'll find a mixture of data hardcoded, in flat files, and in a database, for any reasonably defined program.

Your example code using a bunch of yield statements is one of the most extreme examples of "when all you have is a hammer, everything looks like a nail" that i've seen in a long time. When the answers come in, I think you should read them and think hard about whether a given abstraction you are using is appropriate for the task at hand, as your usage and defending that design means you still need to develop that sense.

0

Having a separate file makes it easier to update (especially if it's just text.) and distribute for updates, but you don't seem to think these values are going to change. Putting it in source code requires a recompile and possible distribution. Again, you don't think it is going to change, so none of this matters until it does.

You've created a scenario where it probably doesn't matter. As another developer who may have to support this app later on (Sort of playing Devil's Advocate here.), I wouldn't expect to have this in the source code. If a database existed, I'd be looking for some sort of currency table/node. Knowing where values come from is a big part of troubleshooting and maintaining code. If that's the only reason, it's good enough for me.

If you've already gone to the trouble of having a database for your app, why not use it?

  • "If that's the only reason, it's good enough for me. " - That can come at a cost. You can now spend time figuring out where the values are, and then spend longer trying to figure how the code knows to find those values, and you're debugging two layers of abstraction instead of one. Not always worth it. – whatsisname Nov 30 '17 at 3:18
  • @whatsisname - There are exceptions, but when if you consistently deal with code that uses the same two layers of abstraction, you get use to it real quick. – JeffO Dec 3 '17 at 15:49

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.