How to properly detect multiple devices failure?

My software has multiple devices connected before starting. I must be sure these devices are working and are well connected. every device has different error ID code so we can see every component that is not working.

When the software will have completly test connection for every devices, we would like to show a message that will show every failing devices:


Unable to connect on one or multiple devices [Error Code: XXXXXXX].
Please look at the user manual at www.xxxx.com/en/UserManual and if
you need further information, contact customer support or mail at
xxxxx@example.com.

For the moment, i am using bitmask but i feel like it's not very "generic" and it kinda hard-coded.

We have made an ID for every class and put a base 2 number like this:

enum eErrorCode
{
    MOTORA    = 0x0000001,
    MOTORB    = 0x0000010,
    SERIAL    = 0x0000100,
    LAN       = 0x0001000,
    MAINBOARD = 0x0010000,
    GENERATOR = 0x0100000,
    MAX = 6
}

So when the Activating function fire, we can add every error:

Int32 ConnectToEveryDevice()
{ 
    Int32 Error = 0;
    foreach(IDevices device in this.Devices)
    {
         Error = Error | device.Connect();
    }
    return Error;
}

For example: If MotorA, Lan and Serial fail, The screen will popup the Hex [Error Code: 0x00001101] so the client can easily find the error in the user manual (online) and if we need to add devices we can do it without needing to change everything. Does anyone have a better or cleaner alternative than this one?

I think your enum is taking naming conventions, making a little paper ball out of it, and throwing it out the window.

An enum is a type. As such, its name should be PascalCase. Enum members are, well, public members of a type. As such, their names should be PascalCase as well.

Consider:

enum ErrorCode
{
    MotorA    = 0x0000001,
    MotorB    = 0x0000010,
    Serial    = 0x0000100,
    Lan       = 0x0001000,
    MainBoard = 0x0010000,
    Generator = 0x0100000,
    Max = 6
}

A number of things are not clear:

• Why use a hexadecimal representation of the values?

• Why is 6 not represented as hex, too?

• Is Max supposed to represent all 6 bit flags turned on?

We have made an ID for every class and put a base 2 number...

No, you haven't. Your numbers are hexadecimal numbers, they're base-16.

Consider:

[Flags]
enum ErrorCode
{
    None = 0,
    MotorA = 1,     // 2^0, or 1 << 0
    MotorB = 2,     // 2^1, or 1 << 1
    Serial = 4,     // 2^2, or 1 << 2
    Lan = 8,        // 2^3, or 1 << 3
    MainBoard = 16, // 2^4, or 1 << 4
    Generator = 32, // 2^5, or 1 << 5
    Max = MotorA | MotorB | Serial | Lan | MainBoard | Generator
}

Take a look at this comment on SO:


Flags itself does nothing. Also, C# does not require Flags per se. But the ToString implementation of your enum uses Flags, and so does Enum.IsDefined, Enum.Parse, etc. Try to remove Flags and look at the result of MyColor.Yellow | MyColor.Red; without it you get "5", with Flags you get "Yellow, Red". Some other parts of the framework also use [Flags] (e.g., XML Serialization). – Ruben Aug 17 '09 at 17:30

This means the [Flags] attribute changes how ToString() represents an ErrorCode value. Your code breaks the type safety and treats the enum as an int, but you could do this as well:

ErrorCode ConnectToEveryDevice()
{ 
    var result = ErrorCode.None;
    foreach(IDevices device in this.Devices)
    {
         result = result | device.Connect();
    }
    return result;
}

Then the return value of this method could be made meaningful to a user, simply with a ToString() call:

var result = ConnectToEveryDevice();
var resultString = result.ToString();

Where resultString holds "MotorA, MainBoard, Generator" if MotorA, MainBoard and Generator are in an error state.

StackExchange Code Review Q#43934, answer score: 4