views:

138

answers:

8

I'm starting on a project that pulls data from and writes back to a legacy system database. I've started on the domain model and am trying to improve this design over past systems, so I'd like some feedback on this one.

This example is arbitrary, so no need for specific advice there, but let's say there's a table in the database called "WorkflowStep" that I'm writing a class for. There's a column in the table called "CurrentStatus" that defines what state the workflow is in. It's stored as a varchar. There are five distinct strings in the entire table for this column, and aren't likely to be changed...values like "Open," "Closed," "On Hold," and so on.

The class needs to track this value, but in what fashion? I could go the easy route and just store it in a string, but that's not very well-defined and I'd envision future developers hunting for distinct values of the string to apply logic against. I could go with an enum to make things more well-defined, but that could lead into switch/case hell all over the place. I've read approaches where engineers make an interface, say "IStatus," and then make concrete classes that represent each possible state of the status, but some other columns in the same situation as this one could have a hundred distinct values, so 100 classes for each state seems like overkill.

My main question: is one approach de-facto better than the others, and if not, what should I consider in order to choose an approach?

Note that the project is still in its infancy and I'm not sure exactly how this "status" attribute of the class will be used. It could be for nothing, or it could prove vital: I'm not sure yet.

+2  A: 

I personally prefer this kind of switch statements

var x = new Dictionary<Status, Action>  
  {Status.Fail, ()=>Console.Writeline("Fail")}
  {Status.Ok, ()=>Console.Writeline("Ok")};  

x[Status.Fail]();
x[Status.Ok]();

Also - you might like this article by Jimmy (in case you will stick with enum fashioned solution).

Arnis L.
Comment by Ryan in the article by Jimmy suggests Jimmy has discovered the Strategy Pattern (http://en.wikipedia.org/wiki/Strategy_pattern).
Ewan Todd
There's nothing new on this planet... ^^
Arnis L.
+1  A: 

I've read approaches where engineers make an interface, say "IStatus," and then make concrete classes that represent each possible state of the status,

That would be the State Pattern.

but some other columns in the same situation as this one could have a hundred distinct values, so 100 classes for each state seems like overkill.

This seems like a separate concern. There's not much to say about it without more details.

Ewan Todd
+1  A: 

I would recommend that you investigate the concept of a "value-Type" as defined in Domain Driven Design. This WorkflowStep thingy sounds like a prime candidate. You don't mention what language or environment you are coding in, but in .Net, if I ended up using this concept, I would create a struct that represented this object, and was coded so that in it's public interface, in use by client code throughout the rest of the system, it appeared as, and would be used as, an enum.

MyTask.WorkFlowStatus = WorkFlowStep.OnHold; 
// creates and assigns new instance of struct

In C#

  public struct WorkflowStep 
  {
     public static readonly WorkflowStep Null   = new WorkflowStep();
     public static readonly WorkflowStep Open   = new WorkflowStep(1);
     public static readonly WorkflowStep Closed = new WorkflowStep(2);
     public static readonly WorkflowStep OnHold = new WorkflowStep(3);
     private int val;
     private bool def;
     public bool HasValue { get { return def; } }
     public bool IsNull   { get { return !def ; } }
     private WorkflowStep ( ) { } // disable public instantiation
     private WorkflowStep (int value) 
     {
       val = value;
       def = true ;
     }
  }

You can type propertys of other clases in your model that need to track this as WorkflowStep. The in-memory footprint of each instance of this struct is very small, the int to hold the value, and a bool to hold it's nullable status. You can add additional methods and overloads to the struct to enable print format outputs, ToString Overloads, whatever you want...

Charles Bretana
+1  A: 

This is a very common issue that depends a lot on understanding the problem. Usually I see this as an intersection between simplicity, readability, extensibility.

  1. Enum / List of constants. This is the simplest route and has good readability if you'll be working in the code much more than in the raw data. Extensibility is not great because you need to refactor to keep this simple.
  2. Strings. The advantage of strings is that you can easily understand the database data at a glance. However, they tend to add cruft to the program code and they're not very extensible either.
  3. A reference table. This is the most extensible of the three by far. Readability is somewhat diminished because now you have to lookup the values in the reference table whenever you want to figure out what's going on. The coding for this is the most complex of the three.

Most simple = Enum. Most readable = Strings. Most extensible = Reference Table.

Which one fits your problem best?

Kai
And even with enums you can store strings in the database so its has the same advantage as Strings without the drawback.
Vincent Ramdhanie
@Vincent Certainly :D You could also use strings or integers for the reference table. There's combinations of these but from my experience those are the basic options.
Kai
+1  A: 

If you are fairly certain that the number of options for the status is not going to increase then an enum would work. I think you can avoid the switch/case problem with some careful design...maybe using a strategy pattern.

On the other hand if the number of options may increase then creating a WorkFlowStatus class would be the way to go. This will allow you to add more statuses without modifying code. I don't think that you have to create a class for each individual status.

Vincent Ramdhanie
+1  A: 

Normally, you'd end up referencing this type of item as an enumeration. If you want to avoid switch/case hell, you could use an Action that mapped to each enumeration - here's an example:

private Dictionary<WorkflowStep, Action> _actions = 
  new Dictionary<WorkflowStep, Action>();

Then assign it with

_actions.Add(WorkflowStep.Open, OpenCommand);
_actions.Add(WorkflowStep.Closed, ClosedCommand);

You would have simple methods like this:

private void OpenCommand()
{
  // Do something
}

private void ClosedCommand()
{
  // Do something.
}

Finally, to invoke it you'd do:

if (_actions[step] != null)
{
  _actions[step]();
}

Note that you could do this with the string - without using the enumeration, but this method helps you to avoid magic strings in your code.

Pete OHanlon
+1  A: 

The state-pattern sounds reasonably.

In case that You'll have more distinct values You could use one of the root implementations, and add some decorator-pattern.

bua
+2  A: 

I don't think there's a definitive answer anywhere for this, but it's a common problem, and the following approach has always served me well:

  1. If your business logic or any code is never going to do anything with the values (comparison, switching based on the value of the enum, etc), keep it as a string.
  2. If you will be doing comparison on it, use an enum. If you have lots of these tables, or the values can change, this is an excellent fit for code generation.
  3. If the values have additional metadata that is relevant to them, create classes that hold this metadata, as well as an enum to create the relevant class (in a factory method). Again, code generation is an excellent fit here.

For something like CurrentStatus, it sounds like option 2 or 3 would be the natural fit. Option 3 would be preferable if any of the following situations apply:

  • You want a "friendly" name for the status that has spaces or non-alpha numeric characters (anything that Enum.ToString()) cannot handle.
  • There is additional logic or properties that "come along" with a certain status. For example, if you have some logic where a certain action could be performed on "OnHold" or "Closed" items but not on "Open" items, it mike make sense to have a "CanDoAction" property on the class.
  • The database you're pulling from has a reference table for statuses with information related to those statuses that you might want to use in your code.
Ryan Brunner
Exactly what I needed, thanks!
Chris