tags:

views:

2525

answers:

7

In the external code that I am using there is enum:

enum En {VALUE_A, VALUE_B, VALUE_C};

In another external code that I am using there are 3 #define directives:

#define ValA 5
#define ValB 6
#define ValC 7

Many times I have int X which is equal to ValA or ValB or ValC, and I have to cast it to the corresponding value of En (ValA to VALUE_A, ValB to VALUEB, etc) because some function signature has enum En. And many times I have to do the opposite operation, translate enum En to ValA or ValB or ValC. I cannot change the signatures of these functions, and there are many such functions.

The question is: How to do the translation? Should I create 2 cast operators, which will be used implicitly? Or should I just have 2 translation functions which will be used explicitly:

En ToEn(int)
int FromEn(En)

Or any other solution?

Thank you.

+1  A: 
Patrick
That could turn into a lot of code. Also, how do you handle overloading methods if you can't change the headers? (Which often happens with libraries.)
Mr.Ree
I can overload methods in another header. But I won't do it because it is indeed a lot of code.
Igor Oks
+1  A: 

You can not overload operators for enums. Or am I missing something? Well, you could create some sort of dummy classes, that would have an implicit constructor taking an int, and then have a cast operator to an enum (and vice versa).

So, the only solution is to have functions. Also, I would make the overloads as Patrick suggests.

Paulius Maruška
+2  A: 

While implicit casting is more convenient than translation functions it is also less obvious to see what's going on. An approach being both, convenient and obvious, might be to use your own class with overloaded cast operators. When casting a custom type into an enum or int it won't be easy to overlook some custom casting.

If creating a class for this is not an option for whatever reason, I would go for the translation functions as readability during maintenance is more important than convenience when writing the code.

mxp
A: 

Converting enum-to-int, e.g. int(VALUE_A), happens automatically/transparently.

Converting int-to-enum, e.g. En(ValA), can benefit from sanity checking to make sure the int value is a valid member of the enum. (Though hopefully the library code doesn't assume its enum values are valid in the first place.)

While it won't help with "int x" cases, you can help somewhat by changing:

#define ValA 5

To:

#define ValA VALUE_A

Provided enum En() is included/defined everywhere, both ValA and *VALUE_A* will work for both foo(int) and bar(En) everywhere automatically/transparently.

You could use:

#ifdef ValA
STATIC_ASSERT( ValA == VALUE_A, ValA_equal_VALUE_A );
#undef ValA
#else
#warning "ValA undefined.  Defining as VALUE_A"
#endif
#define ValA VALUE_A

Where STATIC_ASSERT is something like:

    /* Use CONCATENATE_4_AGAIN to expand the arguments to CONCATENATE_4 */
#define CONCATENATE_4(      a,b,c,d)  CONCATENATE_4_AGAIN(a,b,c,d)
#define CONCATENATE_4_AGAIN(a,b,c,d)  a ## b ## c ## d

    /* Creates a typedef that's legal/illegal depending on EXPRESSION.       *
     * Note that IDENTIFIER_TEXT is limited to "[a-zA-Z0-9_]*".              *
     * (This may be replaced by static_assert() in future revisions of C++.) */
#define STATIC_ASSERT( EXPRESSION, IDENTIFIER_TEXT)                     \
  typedef char CONCATENATE_4( static_assert____,      IDENTIFIER_TEXT,  \
                              ____failed_at_line____, __LINE__ )        \
            [ (EXPRESSION) ? 1 : -1 ]
Mr.Ree
A: 

EDIT: after re-reading the question, I saw that the enumeration was in an external code, so the following solution might not be applicable.

A simple solution would be to set the enumeration values so that they reflect the #defined values:

#define ValA 5
#define ValB 6
#define ValC 7

enum en { VALUE_A = VALA, VALUE_B = VALB, VALUE_C = VALC };

That being done, integer values of the enumerations will be the same as the macro'ed ones, and you'll be able to write code like that:

int x = VAL_B;

switch (x)
{
    case VALUE_A : cout << "A" << endl; break;
    case VALUE_B : cout << "B" << endl; break;
    case VALUE_C : cout << "C" << endl; break;
}

(which will correctly prints B).

Luc Touraille
That's correct, but unfortunately i am unable to change the #defines and the enum.
Igor Oks
+4  A: 

Since you can't just cast here, I would use a free function, and if there are likely to be other enums that also need converting, try to make it look a little bit like the builtin casts:

template<typename T>
T my_enum_convert(int);

template<>
En my_enum_convert<En>(int in) {
    switch(in) {
        case ValA: return VALUE_A;
        case ValB: return VALUE_B;
        case ValC: return VALUE_C;
        default: throw std::logic_error(__FILE__ ": enum En out of range");
    }
}

int my_enum_convert(En in) {
    switch(in) {
        case VALUE_A: return ValA;
        case VALUE_B: return ValB;
        case VALUE_C: return ValC;
        // no default, so that GCC will warn us if we've forgotten a case
    }
}

En enumValue = my_enum_convert<En>(ValA);
int hashDefineValue = my_enum_convert(VALUE_A);
enumValue = my_enum_convert<En>(0); // throws exception

Or something like that - might adjust it if issues arise while using it.

The reason I wouldn't use implicit conversion is that there already is an implicit conversion from En to int, which gives the wrong answer. Even if you can reliably replace that with something which gives the right answer, the resulting code won't look as though it's doing any conversion. IMO this will hinder anyone who later looks at the code more than typing a call to a conversion routine will hinder you.

If you want converting to int and converting from int to look very different, then you could give the template and the function above different names.

Alternatively, if you want them to look the same (and more like a static_cast), you could do:

template<typename T>
T my_enum_convert(En in) {
    switch(in) {
        case VALUE_A: return ValA;
        case VALUE_B: return ValB;
        case VALUE_C: return ValC;
    }
}

int hashDefineValue = my_enum_convert<int>(VALUE_A);

As written, T must have an implicit conversion from int. If you want to support T that only has an explicit conversion, use "return T(ValA);" instead (or "return static_cast<T>(ValA);", if you consider that single-arg constructors are C-style casts and hence impermissible).

Steve Jessop
I like this solution because I might also have other enums that need converting.Why did you choose to have the same name for 2 functions? Maybe having to_enum_convert and from_enum_convert will look clearer?
Igor Oks
Whichever you prefer - I did it this way because I intend my_enum_convert to stand in for static_cast, but with the customised mapping between values, and always converting to int when given an enum parameter.
Steve Jessop
I've edited in an attempt to discuss the options for whether the two directions should look properly different, or properly the same.
Steve Jessop
This is awesome.I didn't get the relation to static_cast until you mentioned it though. Maybe "enum_cast" or "my_enum_cast" would be a better name?
mxp
A: 

I am not sure how the template is helping in translating from any enum to any enum. May be I am missing something. Dont the enums have to have the same members for you to be able to use these functions. Since I do not see how in the return you can convert the int value to the return enum type. Ash

ashutosh