3
\$\begingroup\$

I face a problem I think I am not approaching appropiately.

I have two independent enumerators that both will contain the same members, the only difference being the values given.

I have to be able to parse from one enum type to the other on the fly and I've seen the following code works as intended:

(enumType)Enum.Parse(typeof(enumType), enum1.ToString());

But I feel like there's something I am doing wrong, I believe this code is prone to errors and I feel like I need some help approaching this problem.

The code involved is the following:

public enum AdjustCircuitsCurrent
{
    _2V = 0x0001,
    _1_28V = 0x0002,
    _333mV = 0x0003,
    _0_25A = 0x0004,
    _0_01V = 0x0005,
    _0_1V = 0x006,
    _1V = 0x0007,
}

public enum VerificacionCircuitsCurrent
{
    _2V = 0x000B,
    _1_28V = 0x000C,
    _333mV = 0x000D,
    _0_25A = 0x000E,
    _0_1V = 0x00F,
    _0_01V = 0x0011,
    _1V = 0x0012,
}

public VerificacionCircuitsCurrent ConvertToVerificationCircuit(AdjustCircuitsCurrent circuit)
{
    return (VerificacionCircuitsCurrent)Enum.Parse(typeof(VerificacionCircuitsCurrent), circuit.ToString());
}

Again, this code works as intended, I can convert from one enum to the other, but I feel there's something wrong with it.

\$\endgroup\$
  • \$\begingroup\$ Could you provide a bit of background about why there are two incompatible enums? \$\endgroup\$ – 200_success Dec 1 '15 at 8:23
  • \$\begingroup\$ The engineers use the same register for performing different operations over the same circuit. I have tried naming the enumerators according to the purpose the register values met. \$\endgroup\$ – Oscar Guillamon Dec 1 '15 at 8:26
  • \$\begingroup\$ But why are the values different ? \$\endgroup\$ – Heslacher Dec 1 '15 at 8:28
  • \$\begingroup\$ Also, why do most of the members of these …Current enums look like voltages? \$\endgroup\$ – 200_success Dec 1 '15 at 8:28
  • 1
    \$\begingroup\$ I suspect that there are some serious code quality issues, and that the need to do this conversion is just a symptom of deeper problems. It's hard to help you, though, based on this tiny glimpse into your codebase. \$\endgroup\$ – 200_success Dec 1 '15 at 8:31
3
\$\begingroup\$

There's nothing particularly wrong with the code. It happens often enough that this question has plenty of upvotes.

Marc Gravell gives the exact code that you have. But you can go take a look at the link for ideas about using extension methods to make calling the conversion nicer and possibly adding an "IsDefined" check in case the enums get out of sync sometime in the future.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Yeah, I took a look at it but in this case it doesn't fully apply, since the enums have different values thus making a cast invalid. That's why I am using the Enum.Parse, because I want to use the enumerator name as the parameter for the conversion. Although giving it a second try I found a way to make it better. Just wanted to avoid using out parameters \$\endgroup\$ – Oscar Guillamon Dec 1 '15 at 9:02
  • \$\begingroup\$ I would like to know your opinion on this kind of things, is it bad practice? If so I want to avoid it as much as possible, I don't want constraints given to me derive in bad practice \$\endgroup\$ – Oscar Guillamon Dec 1 '15 at 9:06
  • 2
    \$\begingroup\$ I don't fully understand what you're doing but it seems like you might be better off creating a "Current" class with methods/properties that get the conversion values you need. public static Circuit TwoVolt = new Circuit(0x000B, 0x0001); public class Circuit { public readonly int VerficationValue; public readonly int AdjustmentValue; public Circuit(int verficationValue, int adjustmentValue) { VerficationValue = verficationValue; AdjustmentValue = adjustmentValue; } } \$\endgroup\$ – Kim Dec 1 '15 at 9:32
1
\$\begingroup\$

Your are right, you are listening to your intuition, there is something important to improve in this scenario. Maintaniability. I will expand on on Kim´s comment.

The problem is that if someone else modifies this code in the future and does not know that the two enums really needs to be synced to each other, you will get an error. The problem is that the error will only manifest in in runtime, but just as Kim suggests, you can make the compiler verify this for you by putting both values in the same type and thus informing your fellow maintainers that there is a strong connection between the adjust circuit current and the verification circuit current and the one may not exist without the other.

public class CircuitVoltage
{
    private CircuitVoltage(int adjustCurrent, int verificationCurrent)
    {
    }

    public int AdjustCurrent {get; private set; }
    public int VerificationCurrent { get; private set; }

    public static readonly CircuitVoltage _2V = new CircuitVoltage(0x0001, 0x000B);
    public static readonly CircuitVoltage _1_28V = new CircuitVoltage(0x0002, 0x000C);
    public static readonly CircuitVoltage _333mV = new CircuitVoltage(0x0003, 0x000D);
    public static readonly CircuitVoltage _0_25A = new CircuitVoltage(0x0004, 0x000E);
    public static readonly CircuitVoltage _0_01V = new CircuitVoltage(0x0005, 0x000F);
    public static readonly CircuitVoltage _0_1V = new CircuitVoltage(0x0006, 0x0011);
    public static readonly CircuitVoltage _1V = new CircuitVoltage(0x0007, 0x0012);
}

With this solution you can pass around CircuitVoltage around your implementation and have access to both the adjust current and the verification current and always be sure that you have a correct mapping between them. If someone adds/modifies/removes an adjust current or verification current, they will be forced to do it as a pair.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.