0

I'm trying to make a class using enumerated values for a position of a sensor and I am using vectors with a type int as the input for this function, and I want an enumerated value out. I'm not sure if this code will work or not. I'm not quite sure how to test it.

#include <vector>

place getPos(vector<int>& pin)
    {
        int i;
    for(i = 0; i <= sizeof(pin); i++)
    {
    if (pin[i])
        break;
    }

    place castEnum = (place)i;
    return castEnum;

    }

So this is the update as far as I can gather:

#include <vector>

place getPos(vector<int>& pin)
{
    int i;
    for(i = 0; i <= pin.size(); i++)
    {
        if (pin[i])
            break;
    }
    return static_cast <place> (i);

}

BWE_CO-OP
  • 1
  • 1
  • You could write some test cases, to see if it produces the results you expect. I can see at least one issue. – benjymous Nov 06 '13 at 17:03
  • 3
    `sizeof(pin)` won't do what you think it does. – benjymous Nov 06 '13 at 17:07
  • 1
    If you're not sure how to test it, you're not sure how it should work. First clarify that, and then run it so verify that it actually does that. – Daniel Daranas Nov 06 '13 at 17:08
  • 3
    Another significant issue would be casting values that are simply not defined within the enumeration. That C-style cast has no business in a C++ program regardless. This literally wreaks of an XY problem, where he actual problem is X, but you've identified the problem Y in your solution as what needs fixing. – WhozCraig Nov 06 '13 at 17:08
  • I think you using vector for const size array. consider use of `std::array` and define array size as macro. – SHR Nov 06 '13 at 17:11

3 Answers3

3

An integral value may be cast to an enum by using static_cast:

enum FooType
{
  ftOne = 1,
  ftTwo
};

int main()
{
  const int n = 1;
  FooType ft = static_cast <FooType> (n);
}

Since static_cast is a compile-time operation and n is known only at run-time, if n does not match one of the enumerated values then the resulting value is unspecified. This can be a serious problem.

You will need to determine beforehand that the value you're about to cast is a legitimate value for that enum. When you think about this, usually this defeats the purpose of doing the cast in the first place.

This is often an indication that you're doing something wrong. Why would you need to cast an integral value to an enum? One possible reason is because you pulled that value off of a socket or got it from some other means of interprocess communication. Aside from that no other valid use cases come immediately to mind. This is a classic indication of an XY Problem.

Community
  • 1
  • 1
John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • *"if n does not match one of the enumerated values then the resulting value is unspecified"* This is a slight simplification; the resulting value is unspecified if it's not *in the range* of the enumerators. If the underlying type is fixed, this type defines the range; otherwise, the range is some [0, 2^N] for positive enumerators, where N is based on the min and max enumerator values. – dyp Nov 06 '13 at 17:36
  • @DyP: Are you suggesting that `static_cast(2)` is specified for `enum { foo_a = 1, foo_b = 3 };`? – John Dibling Nov 06 '13 at 17:42
  • I think so; at least I looked into that matter [some time ago](http://stackoverflow.com/a/18195408/420683) and it seemed to me to be well-defined back then. – dyp Nov 06 '13 at 17:44
  • @DyP: That seems very odd to me. Thanks for the clarification. – John Dibling Nov 06 '13 at 17:53
1

Just fixing the obvious:

#include <vector>

place getPos(vector<int>& pin)
{
    int i;
    // sizeof(pin) is a constant representing the size of a vector type
    // Edit: missed that for(i = 0; i <= pin.size(); i++)
    for(i = 0; i < pin.size(); ++i)
    {
        if (pin[i])
            break;
    }
    return place(i);
}

Not so obvious:

You may replace your vector with a std::bitset (or an unsigned integer)
representing the flags in your enum.
0

To address the errors and logical issues:

#include <vector>

place getPos(vector<int>& pin)
{
    int i;
    for(i = 0; i < pin.size(); i++)
   //            ^ Needs to be <, not <=
   //              ^^^^^^^^^^ Needs to be pins.size(), not sizeof(pins) 
    {
        if (pin[i])
            break;
    }

    // alternatively,
    //int i = std::find_if(pins.begin(), pins.end(), [](int a)
    //{
    //    return a != 0;
    //});

    place castEnum = DEFAULT_ENUM_VALUE; // whatever you want for a default enum value
    switch (i)
    {
    case ENUM_VALUE_1:
        castEnum = ENUM_VALUE_1;
        break;
    case ENUM_VALUE_2:
        castEnum = ENUM_VALUE_2;
        break;
    // etc.
    }
    return castEnum;
}

Using the switch statement avoids issues with trying to cast values that are not valid for your enumeration. You will need to define an enumeration value (e.g. INVALID_ENUM = -1) as a default value.

Zac Howland
  • 15,777
  • 1
  • 26
  • 42