Our database has several tables that have a value that can be one of 4 things, so we made these columns instead have an id column with a foreign key to an enum table with an id and a value column to hold those 4 values. When interfacing this through an ORM, I came up with the following enum-like class to manage keeping things in sync:
using DataAccess.ECI.Models;
using System;
using System.Collections.Generic;
using System.Text;
using System.Linq;
namespace DataAccess.ECI
{
public class StatusEnum
{
private string type;
private static Dictionary<string, Status> lookupCache = null;
private static List<StatusEnum> members= new List<StatusEnum>();
private StatusEnum(string type)
{
this.type = type;
members.Add(this);
}
//the text needs to be kept consitent with the Status table
public static readonly StatusEnum ACTIVE = new StatusEnum("ACTIVE");
public static readonly StatusEnum COMPLETED= new StatusEnum("COMPLETED");
public static readonly StatusEnum DELETED= new StatusEnum("DELETED");
public static readonly StatusEnum INPROGRESS= new StatusEnum("INPROGRESS");
public static void Init(ECIContext ctx)
{
lookupCache=new Dictionary<string, Status>();
foreach(StatusEnum member in members)
{
try
{
lookupCache.Add(member.type, ctx.Status.Single(s => s.Name == member.type));
}catch(Exception ex)
{
throw new InvalidOperationException($"No record found with Name {member.type}",ex);
}
}
}
public Status asDBType()
{
if (lookupCache == null) throw new InvalidOperationException("Init(ECIContext) has not been called for this enum");
return lookupCache[type];
}
public override string ToString()
{
return type;
}
}
}
I thought it works pretty well, but I'm concerned about a few things.
- since it's mostly static methods, it forces lots of duplication when other enum tables are added that need the same code wrapper
- if we forget to call init, this error isn't caught until late in the runtime
I would appreciate any comments on the code and suggestions for how to make it better.
EDIT:
for more context, we are using aspnet core, so the context is added in Startup.cs:ConfigureServices(IServiceCollection services) :
...
services.AddDBContext<ECIContext>(options=>
options.UseSqlServer(/*connection string from config*/)
);
...
and Init
is currently called in Startup.cs:Configure(IApplicationBuilder app,...):
...
ECIContext ctx=app.ApplicationServices.GetService<ECIContext>();
StatusEnum.Init(ctx);
...
this enum will be used like
ctx.Document.add(new Document{
PageCount=4,
Status=StatusEnum.ACTIVE.asDBType(),
...other properties...
});
which works because our ORM, entity framework core, adds a field for the foreign key linked table, and stores the id when appropriate
1 Answer 1
So your Init
threw me off, initially I thought this class was doing something, but it was doing something else (not your fault, totally mine).
Here's how I would handle this:
- Get rid of
asDBType
and just return theStatus
, we'll see how in a moment. - Don't call the private member
type
if the database member isName
, call itname
. - Use
CallerMemberNameAttribute
so you don't need a magic string.
You want type-safety at compile time, against something that's inherently unsafe, so we have to cheat quite a bit to make that happen.
We're also going to use some Reflection - doing so means we can eliminate the List<StatusEnum> members
, and replace a lot of this class with a couple quick calls.
public class StatusEnum
{
private static Dictionary<string, Status> _lookup = new Dictionary<string, Status>();
private static Status _get([CallerMemberName]string name = null) =>
_lookup.Length > 0 ? _lookup[name] : throw new InvalidOperationException($"The method '{nameof(Init)}' has not been called.");
// `CallerMemberNameAttribute` gives us the name of whatever
// method or property called this method, which means since
// our property calls `_get()`, we get the name of that
// property implicitly
public static Status ACTIVE => _get();
public static Status COMPLETED => _get();
public static Status DELETED => _get();
public static Status INPROGRESS => _get();
public static void Init(ECIContext ctx)
{
foreach (var prop in typeof(StatusEnum).GetProperties())
{
var name = prop.Name;
// Assuming `ctx.Status` has a `SingleOrDefault`
var status = ctx.Status.SingleOrDefault(x => x.Name == name);
if (status == null) { throw new ArgumentException($"The value '{name}' is not a valid '{nameof(Status)}'."); }
_lookup.Add(name, status);
}
}
}
Realistically, your StatusEnum
class is a bunch of helper stuff, so you don't really need instance members, and you don't really want to keep an instance of this class, you care about the Status
class. This is similar to how I do Setting
objects in Entity Framework, though my setup was more complex.
Your version requires adding a property with a self-referencing name, this version eliminates that.
public static Status ONHOLD => _get(); public static readonly StatusEnum ONHOLD = new StatusEnum("ONHOLD");
Also, in your version, C#6.0 introduced nameof()
which would allow you to safely initialize:
public static readonly StatusEnum ACTIVE = new StatusEnum(nameof(Active));
The other advantage to this version is you'll always be sure that you have a valid Status
object, no need to asDBType
, which means we can make this generic for your other situations:
public abstract class ItemizedEnum<TItem, TClass>
where TClass : ItemizedEnum<TItem, TClass>
{
private static Dictionary<string, TItem> _lookup = new Dictionary<string, TItem>();
protected static TItem Get([CallerMemberName]string name = null) =>
_lookup.Count > 0 ? _lookup[name] : throw new InvalidOperationException($"The '{nameof(Init)}' method MUST be called first.");
protected abstract TItem GetFromDatabase(ECIContext ctx, string name);
public static void Init(ECIContext ctx)
{
var classInstance = Activator.CreateInstance<TClass>();
foreach (var prop in typeof(TClass).GetProperties())
{
var name = prop.Name;
var obj = classInstance.GetFromDatabase(ctx, name);
if (obj == null || obj.Equals(default(TItem)) == true) { throw new ArgumentException($"The value '{name}' is invalid for '{typeof(TClass).Name}'"); }
_lookup.Add(name, obj);
}
}
}
Finally, in StatusEnum
:
public class StatusEnum : ItemizedEnum<Status, StatusEnum>
{
public static Status ACTIVE => Get();
public static Status COMPLETED => Get();
public static Status DELETED => Get();
public static Status INPROGRESS => Get();
protected override Status GetFromDatabase(ECIContext ctx, string name) =>
ctx.Status.FirstOrDefault(x => x.Name == name);
}
This decreases the level of redundancy, the class only needs to implement a GetFromDatabase(ECIContext, string)
method, and we now no longer care about any of that in StatusEnum
itself, just what values it contains. We can then build other ItemizedEnum
classes:
public class StateEnum : ItemizedEnum<State, StateEnum>
{
protected override State GetFromDatabase(ECIContext ctx, string name) =>
ctx.State.FirstOrDefault(x => x.Title == name);
public static State OtherTest => Get();
}
And we get the same type-safety. Now a test of your live examples might be:
public class StatusEnum : ItemizedEnum<Status, StatusEnum> { protected override Status GetFromDatabase(ECIContext ctx, string name) => ctx.Status.FirstOrDefault(x => x.Name == name); public static Status Test => Get(); } public class StateEnum : ItemizedEnum<State, StateEnum> { protected override State GetFromDatabase(ECIContext ctx, string name) => ctx.State.FirstOrDefault(x => x.Title == name); public static State OtherTest => Get(); } StatusEnum.Init(new ECIContext()); StateEnum.Init(new ECIContext()); Console.WriteLine(StatusEnum.Test.Name); Console.WriteLine(StateEnum.OtherTest.Title);
-
\$\begingroup\$ this looks like it solves my first concern nicely, but I realized I only need this approach for one of the enums, where LogLevel needs an explicit ordering and I don't want to tie it to the id, the rest I should be able to just use the id directly \$\endgroup\$Austin_Anderson– Austin_Anderson2017年09月26日 15:08:15 +00:00Commented Sep 26, 2017 at 15:08
Explore related questions
See similar questions with these tags.