7

I'm trying to solve a dilemna that has been nagging me for the last few months.

My colleagues and I completely disagree on a technical problem, and I would like our beloved community's opinion on the matter.

In a nutshell:

Is it preferable to use enumerations (with potentially attributes on them such as "Description"), or use entities (with Name and Description properties)?

In details:

In our domain model, we've got a lot of mini-entities which only contain an Id, a Name, and a Description. 95% of the time, the Description is equal to the Name.

For the sake of the explanation I'm going to take one of the many example: in our Security entity, we have an AssetClass property. An AssetClass has a static list of values ("Equity", "Bond", etc.) and does not change from the interface or anything else.

The problem with that, is when you want to get all securities with an asset class of "Bond" say, NHibernate will have to join on the AssetClass table... and considering AssetClass is not the only property like that, you can imagine the performance impact of all those joins.

Our current solution: (which I disagree with):

We have in our code some hard-coded instances of AssetClass with all their respective values and Ids (i.e. Equity with Id of 1, Bond with Id of 2 etc.), which match what's in the database:

public partial class AssetClass
{
    static public AssetClass Equity = new AssetClass(1, "Equity", "Equity");
    static public AssetClass Bond = new AssetClass(2, "Bond", "Bond");
    static public AssetClass Future = new AssetClass(3, "Future", "Future");
    static public AssetClass SomethingElse = new AssetClass(4, "Something else", "This is something else");
}

We also made a special NHibernate type (code below if you are interested) that allow us to avoid NHibernate doing a join by loading that hard-coded instance instead of going to the database to get it:

using System;
using System.Data;
using System.Data.Common;
using NHibernate.Dialect;
using NHibernate.SqlTypes;
using NHibernate.Type;

namespace MyCompany.Utilities.DomainObjects
{
    public abstract class PrimitiveTypeBase<T> : PrimitiveType where T : class, IUniquelyNamed, IIdentifiable
    {
        private readonly PrimitiveTypeFactory<T> _factory;

        public PrimitiveTypeBase() : base(SqlTypeFactory.Int32)
        {
            _factory = new PrimitiveTypeFactory<T>();
        }

        public override string Name
        {
            get { return typeof(T).Name; }
        }

        public override Type ReturnedClass
        {
            get { return typeof(T); }
        }

        public override Type PrimitiveClass
        {
            get { return typeof (int); }
        }

        public override object DefaultValue
        {
            get { return null; }
        }

        public override void Set(IDbCommand cmd, object value, int index)
        {
            var type = value as T;
            var param = cmd.Parameters[index] as DbParameter;
            param.Value = type.Id;
        }

        public override object Get(IDataReader rs, int index)
        {
            return GetStaticValue(rs[index]);
        }

        public override object Get(IDataReader rs, string name)
        {
            return GetStaticValue(rs[name]);
        }

        private T GetStaticValue(object val)
        {
            if (val == null)
            {
                return (T)DefaultValue;
            }

            int id = int.Parse(val.ToString());
            T entity = _factory.GetById(id); // that returns, by reflection and based on the type T, the static value with the given Id

            if (entity == null)
            {
                throw new InvalidOperationException(string.Format("Could not determine {0} for id {1}", typeof (T).Name, id));
            }
            return entity;
        }

        public override object FromStringValue(string xml)
        {
            return GetStaticValue(xml);
        }

        public override string ObjectToSQLString(object value, Dialect dialect)
        {
            var type = value as T;
            return type.Id.ToString();
        }
    }
}

My solution: (which I agree with :-))

Replacing those entities by enums, and if we ever need a Description field, use an attribute. We could also have a constraint on the database to make sure you can't store random values that don't match an enum.

Their rational against using enumerations:

  • This is not an object, so you can't extend it, it's not object oriented, etc.
  • You can't get the description easily, or have "proper english" names (with spaces or symbols) such as "My Value" (which on an enum would be "MyValue")
  • Enums sucks
  • Attribute sucks

My rational against our current solution:

  • We can have a mismatch between the ids in the code and what's in the database
  • It's a lot harder to maintain (we need to make absolutely sure that every hard-coded values we have are also there in the database)
  • Attributes and enums don't suck if used properly and for static values like these
  • For "proper english" names, we can also use an attribute, with some extension method to consume it.

Now, what do YOU think?

Antoine Jaussoin
  • 5,002
  • 4
  • 28
  • 39

3 Answers3

7

Personally I prefer the first solution, possibly with an additional property which returns all the values.

It's much more OO - enums are basically "named numbers", that's all. Everywhere else in code, state is stored in properties - so why use attributes instead? As Eric Lippert wrote in a blog post comparing the two, "Attributes are facts about the mechanisms". You're basically using them as a way of providing data about values instead, and that just feels wrong to me.

Your first two objections to using POCOs in terms of the potential mismatch between code and the database doesn't ring true either - because they're exactly the same for enums as well, aren't they? Besides, it's very easy to write a test which validates the data - you could even do so on startup if you wanted.

It's not clear what the rest of your AssetClass does, but if it only has a private constructor, then you get a lot of the benefits of enums in terms of a limited, well-known set of values, but without the problem that enums are basically just numbers.

In fact, the POCO solution is better than the enum in terms of value limitation - because the only "invalid" POCO value is null, whereas it's easy to come up with an invalid enum value:

FooEnum invalid = (FooEnum) 12345;

Are you going to check for that everywhere? Generally null references go bang considerably earlier than invalid enum values, and are also easier to check for.

One downside I can think of in the POCO approach is that you can't switch on it. There are ways around that, but they're not terribly pleasant - you'd basically have to also have a set of well-known numbers (which could be an enum, of course) so that some property would return that and you could switch on it.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Many thanks for your reply! I think my choice of AssetClass as an example was probably not very wise (because it feels like a proper Entity indeed), but what about something like a 'Status', which only has 'Estimated', 'Confirmed', etc. as values? Why bother storing those static-and-will-never-change values in the database, while what they really represent is a "flag"? Now I can really see your point, and I'm starting to understand my colleagues point of view better... Also: I disagree with your value limitation point, since I'd be storing my enum as strings in the DB – Antoine Jaussoin Aug 19 '11 at 09:10
  • @Antoine: If they're *just* a single numeric value, then it's more reasonable to use an enum. You'd *still* need to validate that you had the right data though. If you were using the POCO solution already in other places, I *might* use that even for just a single numeric value, for the sake of consistency and the possibility of adding behaviour and more information later on. – Jon Skeet Aug 19 '11 at 09:12
  • They are really single numeric values, the only exception is the "proper english" name you can't have directly from an Enum value (Actually on some past projects I wrote some code that would automagicaly convert a "ThisIsAValue" into "This Is A Value"), which is why I used to use attribute for this. But I now understand better my colleagues point of view, and I will tell them they won the argument :) – Antoine Jaussoin Aug 19 '11 at 09:20
2

I don't really like either options because of potential mismatch of ID's between the code and database. You actually pointed this out, but for some reason seem to think this issue will disappear if you used enums, when in actual fact you'll have the exact same thing?

If I had to choose between the two I'd pick the current implementation for the reasons Jon Skeet has mentioned.

However, I think the best approach is just create a normal class and fetch the items from the database, and program against attributes. I wrote this a while ago to explain what I mean: Program against attributes

David Masters
  • 8,069
  • 2
  • 44
  • 75
  • I just read your article, and it's brilliant. It could certainly apply to some of our cases, so thanks for that!!! Back to my question, I disagree on the mismatch part: I would not store integers in the database, but the string representation of the enum, coupled with a constraint on the column to the list of values of the enum. But Jon Skeet had indeed a lot of points I didn't realise, and I've come to peace with the first solution :) – Antoine Jaussoin Aug 19 '11 at 09:29
  • No probs :). I'd just add that personally I HATE enums. There are probably some circumstances where their use is justified but in general, enums are evil! http://www.planetgeek.ch/2009/07/01/enums-are-evil/ http://www.colourcoding.net/blog/archive/2009/07/25/enums-are-evil-the-state-pattern.aspx – David Masters Aug 19 '11 at 09:35
1

I personally prefer storing enums as strings in the database. Any additional information you might need could go in it's own table with the enum type and value as key. I would not store the description as an attribute though. Also it is trivial to convert Enum.SomeValue to a string containing "Some Value"

I prefer enums mainly because they give you compiler support and make it easy to change add new values with simple refactoring where string values can make some nice hidden bug when values change.

This

if(obj.EnumValue == Enum.Value)

Is better than this

if(obj.Value == "SomeValues")

I use GenericEnumMapper which comes with fluent-nhibernate but if you are using hibernate you can use EnumString type. This gives a nice way to have the enums as strings in the database so you data is readable for people without access to the code.

Yavor Shahpasov
  • 1,453
  • 1
  • 12
  • 19