3

I'm wondering what is the best practice for holding immutable data, more specifically in C#.

For instance, lets say I have an immutable object called Foo.

public sealed class Foo {
    public int Bar { get; }

    public Foo(int bar) {
        Bar = bar;
    }
}

Currently, I have been holding these objects in a static class like so.

public static class FooList {
    private static List<Foo> fooList;

    static Foo {
        fooList = new List<Foo>();
        fooList.Add(new Foo(1));
        fooList.Add(new Foo(2));
    }

    public static Foo GetFoo(int index) {
        if (index >= 0 && index < fooList.Count) {
            return fooList[index];
        }

        // I don't know if this is right, but besides the point
        throw;
    }
}

From there, any class can have their own reference to a Foo object by calling GetFoo(int). However, I have been reading up lately that static classes should be avoided due to them being hard to maintain (I don't know the specific reason, but I would assume it is because they can't be instantiated).

If this is the case, what would be the best practice for holding immutable data in this situation? I am aware of the Singleton Pattern, but I don't know if this is the best practice for this situation.

  • Do you need to globally store the Foo-objects? If so, why? If not, @NathanCooper's awnser will help you avoid the problem. – Kasper van den Berg Aug 22 '15 at 9:17
  • For my specific problem, I will have a lot of Foo objects, and even more objects using Foo objects, many using the same Foo object. At the moment, if I were to use Nathan's suggestion, I would need to declare a new Foo object which would be the same for some objects, instead of referencing one Foo object. – Hayden Aug 22 '15 at 9:20
  • 1
    You are not storing data in an static class. You are defining constants (or alike). If the data is not in persistent storage (e.g. disk files, database), the term store is missleading – Javier Aug 22 '15 at 9:37
  • I've stored this data on disk, then loaded the data to these classes. I understand though the misuse of terminologies though and will edit the question. – Hayden Aug 22 '15 at 9:39
  • That is better. Anyway I fear your question might have different answers depending on your specific circunstances and personal preferences (which is something about mods might frown upon). ++ The Singleton pattern is discussed as I told to Nathan. In your case you are using it practically as if they were constants, in which I see it a proper use and not a big concern for TDD, but that of course is just my shallow and uninformed impression. – Javier Aug 22 '15 at 9:48
4

Fundamentally, there's no problem described in the question that actually needs a solution. You've given no reason for a Foo holder to exist, or what it should be able to do. So really, FooList is completely useless.

If you need a Foo, there's a language feature for that- new Foo(1). There's no reason for any additional holding to be baked in to the Foo class.

Edit: You can use a really simple non-static cache and simply pass around the cache.

public class FooCache {
    private Dictionary<int, Foo> cache = new Dictionary<int, Foo>();
    public FooCache() {}
    public Foo GetFoo(int i) {
        if (!cache.ContainsKey(i))
            cache[i] = new Foo(i);
        return cache[i];
    }
}

This more directly holds the Foo objects that you actually want, instead of newing up a pre-set list and then just hoping that they're correct, it's a lot more transparent, you can still use new Foo if you need to, and you can have several instances of it if you want to (e.g. unit tests).

|improve this answer|||||
  • I should of added it to Nathan's comment instead of my question comments, but the "Foo" objects I am using are large, and some of the objects need to use the same objects. Using new will create a new Foo object, whereas I want to have a reference to that object as I am on tight memory restrictions. – Hayden Aug 22 '15 at 10:11
  • Did you use a memory profiler to prove that the Foo objects are the source? – DeadMG Aug 22 '15 at 10:12
  • I have. Instead of using new Foo, is there a way I can improve the design? – Hayden Aug 22 '15 at 10:13
  • I quite like this idea of caching. I'll accept it due to being an answer with a design pattern. – Hayden Aug 22 '15 at 10:27
  • Design pattern? It's a trivially obvious use of a trivial structure. – DeadMG Aug 22 '15 at 10:29
8

The problem is that you've effectively created a global variable for accessing these objects, the Foolist class. This has drawbacks for things like testability and is in general just poor design.

Global state doesn't help make code easy to read and the consuming classes can never be isolated from the Foos (look up mocking) d when testing.

Avoid the static and pass either the foos or foolists around as instance variables.

|improve this answer|||||
  • The big frown upon singletons is discussed in great extent in SO. Singletons and statics are perfectly good and safe in some cases, IMVHO. – Javier Aug 22 '15 at 9:33
  • That's true, it's just that the number of cases is about 2 in total, all of which are handled for you at the OS level. However, the fact that his is completely immutable IMO prevents most of the disadvantages of statics/singletons, and is really a separate matter from normal singletons. – DeadMG Aug 22 '15 at 10:07
2

The benefit of immutability is that it doesn't matter as much what you do with your references to it. Globals, static classes, and singletons are much less problematic than their mutable counterparts, because you don't have to synchronize mutations and track the order dependencies that creates. However, they still have the issues with mocking out for unit testing.

On the other end of the spectrum, you can have as many references as you want, or even outright copies. This is because you don't have to worry about propagating any changes to the other copies, since there are no changes. This is the hardest part for people to get used to, if they are accustomed to mutability, but is usually the easiest to maintain, and retains the benefits for mocking in unit tests.

|improve this answer|||||
  • Thanks for the answer, immutability is what I am aiming for in this. – Hayden Aug 23 '15 at 23:41
0

(This entry is a wiki. Please feel free to improve this answer)

Here is argumented how using static for immutable might be preferable, and related concepts.

  • Constants themselves are defined as global. Objects that wont change not only during runtime, but wont have alternatives values (like a filesystem path might change depending configuration) a priori will be just as good to be defined as static.
  • These would be instances on indexed values that would not change between executions:
    • A Cache of precomputed values (e.g. like some graphic engines hold trigonometric tables , especially in early 90's when there was no floating point in processor).
    • Like a FlyWeight pattern, where we would be attempting to save memory using canonical instances rather than having many objects with the same value. For example if we load "Smith" surname 10,000 times, we might either keep in memory that value 10,000 times, or keep the first instance in a dictionary, and reference that same canonical instance for every posterior duplicate.
  • There is ongoing debate about static values being something to avoid (as in this popular question in Stack Overflow about the Singleton Pattern)

    • The alternative would be to create an instance that encapsulates all this values (and therefore passing that context as a parameter to any method that needs it).
    • In Test Driven Development that would allow to use custom instances for testing. However, as we talk of immutable data, that might either be an "universal constant", or some custom data that might be loaded as easily from an alternative configuration file.
    • That passing the context as a parameter, might end up in awkward situations, where one or many huge context/component objects are passed along methods, without being very clear what is their use (an extreme case of Stamp Coupling). Expressed a bit poignantly:

    I have seen several developers go to extraordinary lengths to avoid a global because they considered using one as an admission of failure..

  • 3
    I'm not sure how to improve this answer other than deleting it outright, as it appears to be a discussion. – DeadMG Aug 22 '15 at 10:21
  • Discussion is not bad if fact based. Even the official help allows for fact based opinion/discussion. – Javier Aug 22 '15 at 10:27
  • 1
    The only thing that help says about discussion is a) don't do it and b) do it in chat. It says nothing about posting discussions as answers. – DeadMG Aug 22 '15 at 10:34
  • 1
    @Snowman As futile as it might seem, I reestructured the answer. As advised in chat, Comunity Wiki is not a not fit for straight affirmations "It is alright to use static", and anything stated should have as solid grounds as possible. Softened heading and affirmations. Removed the hardly related Secundum Quid. Added relevant reference to Cache use case. – Javier Aug 25 '15 at 1:36

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.