Enum Overuse?

Question

I'm not sure if I am abusing Enums here. Maybe this is not the best design approach.

I have a enum which declares the possible parameters to method which executes batch files.

public enum BatchFile
{
    batch1,
    batch2
}

I then have my method:

public void ExecuteBatch(BatchFile batchFile)
{
    string batchFileName;
    ...
    switch (batchFile)
        {
            case BatchFile.batch1:
                batchFileName = "Batch1.bat";
                break;
            case BatchFile.batch2:
                batchFileName = "Batch2.bat";
                break;
            default:
                break;
        }
    ...
    ExecuteBatchFile(batchFileName);
}

So I was wondering if this is sound design.

Another option I was thinking was creating a Dictionary<> in the constructor like this:

Dictionary<BatchFile, String> batchFileName = new Dictionary<BatchFile, string>();
batchFileName.Add(BatchFile.batch1, "batch1.bat");
batchFileName.Add(BatchFile.batch2, "batch2.bat");

Then instead of using a switch statement I would just go:

public void ExecuteBatch(BatchFile batchFile)
{
    ExecuteBatchFile(batchFileName[batchFile]);
}

I'm guessing the latter is the better approach.

Answer 1

Is it really necessary that ExecuteBatch works on limited number of possible file names only? Why don't you just make it

public void ExecuteBatch(string batchFile)
{
    ExecuteBatchFile(batchFile);
}

Answer 2

I think the latter approach is better because it separates out concerns. You have a method which is dedicated to associating the enum values with a physical path and a separate method for actually executing the result. The first attempt mixed these two approaches slightly.

However I think that using a switch statement to get the path is also a valid approach. Enums are in many ways meant to be switched upon.

Answer 3

The problem with the latter case is if something passed an invalid value that is not inside the dictionary. The default inside the switch statement provides an easy way out.

But...if you're enum is going to have a lot of entries. Dictionary might be a better way to go.

Either way, I'd recommend some way to provide protection of the input value from causing a bad error even in ammoQ's answer.

Answer 4

I would personally use a static class of constants in this case:

public static class BatchFiles
 { 
   public const string batch1 = "batch1.bat";
   public const string batch2 = "batch2.bat"; 
 }

Answer 5

I'd probably go for a design along these lines:

public interface IBatchFile
{
    void Execute();
}

public class BatchFileType1 : IBatchFile
{
    private string _filename;

    public BatchFileType1(string filename)
    {
        _filename = filename;
    }

    ...

    public void Execute()
    {
        ...
    }
}

public class BatchFileType2 : IBatchFile
{
    private string _filename;

    public BatchFileType2(string filename)
    {
        _filename = filename;
    }

    ...

    public void Execute()
    {
        ...
    }
}

In fact, I'd extract any common functionality into a BatchFile base class

Answer 6

The second approach is better, because it links the batch file objects (enums) with the strings..

But talking about design, it would not be very good to keep the enum and the dictionary separate; you could consider this as an alternative:

public class BatchFile {
    private batchFileName;

    private BatchFile(String filename) {
        this.batchFileName = filename;
    }
    public const static BatchFile batch1 = new BatchFile("file1");
    public const static BatchFile batch2 = new BatchFile("file2");

    public String getFileName() { return batchFileName; }
}

You can choose to keep the constructor private, or make it public.

Cheers,

jrh.

Answer 7

Using enums is ok if you don't need to add new batch files without recompiling / redeploying your application... however I think most flexible approach is to define a list of key / filename pairs in your config.

To add a new batch file you just add it to the config file / restart / tell your user the key. You just need to handle unknown key / file not found exceptions.

Answer 8

What if you suddenly need a third batch file? You have to modify your code, recompile your library and everybody who uses it, has to do the same.

Whenever I find myself writing magic strings that might change, I consider putting them into an extra configuration file, keeping the data out of the code.

Answer 9

The first solution (the switch) is simple and straight forward, and you really don't have to make it more complicated than that.

An alternative to using an enum could be to use properties that returns instances of a class with the relevant data set. This is quite expandable; if you later on need the Execute method to work differently for some batches, you can just let a property return a subclass with a different implementation and it's still called in the same way.

public class BatchFile {

   private string _fileName;

   private BatchFile(string fileName) {
      _fileName = fileName;
   }

   public BatchFile Batch1 { get { return new BatchFile("Batch1.bat"); } }
   public BatchFile Batch2 { get { return new BatchFile("Batch2.bat"); } }

   public virtual void Execute() {
      ExecuteBatchFile(_fileName);
   }

}

Usage:

BatchFile.Batch1.Execute();

Answer 10

If you want to use an enum then you may want to consider utilising attributes so you can store additional inforation (such as the file name) against the elements.

Here's some sample code to demonstrate how to declare the attributes:

using System;

public enum BatchFile
{
    [BatchFile("Batch1.bat")]
    batch1,
    [BatchFile("Batch2.bat")]
    batch2
}

public class BatchFileAttribute : Attribute
{
    public string FileName;
    public BatchFileAttribute(string fileName) { FileName = fileName; }
}

public class Test
{
    public static string GetFileName(Enum enumConstant)
    {
     if (enumConstant == null)
      return string.Empty;

     System.Reflection.FieldInfo fi = enumConstant.GetType().GetField(enumConstant.ToString());
     BatchFileAttribute[] aattr = ((BatchFileAttribute[])(fi.GetCustomAttributes(typeof(BatchFileAttribute), false)));
     if (aattr.Length > 0)
      return aattr[0].FileName;
     else
      return enumConstant.ToString();
    }
}

To get the file name simply call:

string fileName = Test.GetFileName(BatchFile.batch1);