Enum or Constant

Question

I'm writing a SDK for a NFC device in .NET so I don't have to import the SDK from C++. Right now I'm working on the ISO14443-3 part which is just simple Halt, Request, and Anticollision commands. The communication part between the device and computer is simple enough so I'm not going to post any of that. Just know that it is a serial device and that the command I send to it gets built before I write it to the SerialPort.

We have 2 different NFC devices with completly different SDK's. I plan on making them identical, but when I first started I was main concerned with only one of them and I was basing all my methods off of the SDK that came with the device. Note that this is not a debate about if I should use the SDK or not. When I first started I figured that I would only have 1 method with a simple structure. It looked like this. private void BuildAndSendCommand(MasterRDCommands command, params byte[] data)

MasterRDCommands is a simple enum. I decided that was a bad idea when I started work on the sound and light commands.. it was super hard to read something like

nfc.BuildAndSendCommand(MasterRDCommands.SetLED, RFIDLED.Blue, 0x01, 0x10);

it's like..HUH???

so I made the BuildAndSendCommands private and made methods to make the code more clear.. Now I have the method signature like this..

public void SetLED(RFIDLED led, byte flashes, byte duration)

that sure make it much nicer at the top most level, but the middle man I'm concerned if I should still use a enum. I feel that it would much more clean if I would just put a region at the bottom or top of my code with a few private constants so that things like the RATS command would switch from

    public byte[] SendRATS_TypeA()
    {
        BuildAndSendCommand(MasterRDCommands.RATS);
        byte[] RATS = GetResponse(10);
        return RATS;
    }

to say something like this

    private const byte RATScmd = 0x1F;
    public byte[] SendRATS_TypeA()
    {
        BuildAndSendCommand(RATScmd);
        byte[] RATS = GetResponse(10);
        return RATS;
    }

it's not much different, but I don't plan on exposing any of the commands from my Enum to the user since most if not all of the commands require a certain order. Where as the LED example is a good example (to me atleast) of when to use a enum. The user has to choose a very narrow set of LED's.

in the end the user still only sees the few methods that I mark as public and would still never know if I ever deleted the Commands enum or not. What do you think? Keep them or remove them?

Answer 1

I've never been a fan of a global constants file. It's a good idea to keep enums defined close to where they are needed. Makes it a little more apparent how enum is used. This helps keep the code clean as you mentioned, and also helps improve maintainability, since there's not a long, master list of enums that need to be mentally processed in order to confidently make a change.

On a side note, I would go ahead and put them in separate files. This will also help with maintainability later, in case a move becomes necessary.

If you can eliminate the enum and just use a private variable, then you've made it even better. Code is more readable, and it doesn't imply that the particular value is shared across larger portions of code.

Answer 2

I think you should keep the enums, they should be used when we need to choose from a set of values, as

• They are self documenting
• IDE will autmotically pop up the available options
• Have better maintainability

We should use consts when we have a single value and it can't be changed. These are read only variables.

Answer 3

I think that using an enum is better. The main reason is that it it more type-safe.

When a parameter is byte, you can pass any numeric value in the byte range to it or a constant that logically belongs to another enum.

With enums, such accidents are not possible*, though you still can pass any value to it if you need to by using casting. (This can be useful in some relatively rare cases; most of the time, it's just annoying.)

* There is one exception, 0 is valid value for any enum. This means you can write BuildAndSendCommand(0), no matter whether the parameter is a byte or an enum. It's good to keep this in mind when debugging weird enum-related errors.

Answer 4

Clean interface

nfc.BuildAndSendCommand(MasterRDCommands.SetLED, RFIDLED.Blue, 0x01, 0x10);

it's like..HUH???

Some issues with the interface:

• Many parameters: usually means parameters wants to be extracted into a domain class.

• BuildAndSend... : Method name of form Verb1AndVerb2 usually indicates either violation of single responsibility principle, or leaking of implementation details to interface.

• ...SendCommand : VerbNoun(x, y, z) form indicates unnecessary verbosity if x is of type Noun and y,z are options. Or x, y, z are components of a domain object Noun and interface should be changed to Verb(new Noun(x, y, z)). (See Many Parameters above) In your case it looks like the second. (MasterRDCommands.SetLED, RFIDLED.Blue, 0x01, 0x10) is your command and MasterRDCommands.SetLED is your Opcode. I'm thinking chip instructions here.

• ... , 0x01, 0x10) : This is called primitive obsession. You can overcome this by encapsulating values in domain value objects, or Enums where suitable. eg compare with SetLED(LEDColor.Blue, new Flashes(true), new Duration(0x10)); It also ensures swapping parameters by mistake is compilation error.

Usually, when no of handler methods is manageable and new commands being added to interface does not present much challenge you would collect above handling to an interface s.a. this:

interface ICommandHandler {
    void SetLED(LedColor ledColor, Flashes flashes, Duration duration);
    void SetLED(Flashes flashes, Duration duration);

    void RATS();

            // etc ........
}

However if number of different commands is large you might want to use Command pattern to keep the interface simple.

// high level interface
interface ICommand
{
}

class RATS : ICommand
{
}

class SetLED : ICommand
{
    public bool Flashes {get; set;} 
    public byte Duration {get; set;}
}

interface ICommandHandler {
    void Handle<T> (T command) where T : ICommand;
}

This interface can be used like this:

                    // example
        ICommandHandler sender = new CommandSender(serializer, byteLevelIO);

        sender.Handle(new RATS());
        sender.Handle(new SetLED(){Flashes = true, Duration = (byte)10});

Ideally commands would be immutable and their fields would be domain value types, but I intentionally left above not as such to demonstrate another readability option.

new SetLED(){Flashes = true, Duration = (byte)10}

vs

new SetLED(new Flashes(true), new Duration(10))

This option saves some typing in exchange for losing some type safety.

Opcodes are a concern of lower level and can be defined elsewhere like this:

enum Opcodes : byte
{
            // opcodes from datasheet
    RATS = 0x1F,
    SetLED = 0x20,
            // .......
}