'Using strings instead of enums?

Is it common place to use a string for comparison as opposed to an enum?

c++sharpmethods


Solution 1:[1]

At the very least, the strings should be declared as constants (or perhaps readonly fields) somewhere, instead of spread out through the code. However, this looks like the schoolbook example for when to use an enum.

public enum ObjectType
{
   UAV,
   Entity,
   // and so on
}

Solution 2:[2]

I am aware about your context, but as a first step you can just refactor this way:

Step 1

if (typeOfObject == "UAV")
{
    DoSomeWork(_stkObjectRootToIsolateForUavs);
}
else if (typeOfObject == "Entity")
{
    DoSomeWork(_stkObjectRootToIsolateForEntities);
}

private void DoSomeWork(IAgStkObject agStkObject)
{
    IAgStkObject stkObject = agStkObject.CurrentScenario.Children[stkObjectName];
    IAgDataProviderGroup group = (IAgDataProviderGroup)stkUavObject.DataProviders["Heading"];
    IAgDataProvider provider = (IAgDataProvider)group.Group["Fixed"];
    IAgDrResult result = ((IAgDataPrvTimeVar)provider).ExecSingle(_stkObjectRootToIsolateForUavs.CurrentTime);

    stkObjectHeadingAndVelocity[0] = (double)result.DataSets[1].GetValues().GetValue(0);
    stkObjectHeadingAndVelocity[1] = (double)result.DataSets[4].GetValues().GetValue(0);
}

Then consider replasing if's with switch:

Step 2

switch (typeOfObject)
{
    case "UAV":
        DoSomeWork(_stkObjectRootToIsolateForUavs);
        break; 
    case "Entity":
        DoSomeWork(_stkObjectRootToIsolateForEntities);
        break;
    default:
        throw new NotImplementedException():
}

This can be even better when using enums.

Solution 3:[3]

To add to @Restuta's answer, I'd use a 

IDictionary<MyEnumifiedString, Action<IAgStkObject>>

to get rid of that if.

Solution 4:[4]

I'd agree with @Frederik that this seems a perfect case for using enums, but it could be that the only thing you can get out of the application is a string. In which case your example is perfectly OK.

Oh yes - and make sure you have the string constants defined in one place, preferably a config file so that if they change the other application you don't have to recompile yours.

Solution 5:[5]

Regarding your first question I will always use a defined type to store the strings simply to have one location for change if needed.

So for your example i would have the following

public sealed class RootTypes
{
    public const string Entity = "entity";
    public const string UAV = "uav";
}

Your code then updates to this 

    typeOfObject = typeOfObject.ToLower();
    if (typeOfObject == RootTypes.UAV)
    {
        stkUavObject = _stkObjectRootToIsolateForUavs.CurrentScenario.Children[stkObjectName];
        var group = (IAgDataProviderGroup) stkUavObject.DataProviders["Heading"];
        var provider = (IAgDataProvider) group.Group["Fixed"];
        IAgDrResult result = ((IAgDataPrvTimeVar) provider).ExecSingle(_stkObjectRootToIsolateForUavs.CurrentTime);
        stkObjectHeadingAndVelocity[0] = (double) result.DataSets[1].GetValues().GetValue(0);
        stkObjectHeadingAndVelocity[1] = (double) result.DataSets[4].GetValues().GetValue(0);
    }
    else if (typeOfObject == RootTypes.Entity)
    {
        IAgStkObject stkEntityObject = _stkObjectRootToIsolateForEntities.CurrentScenario.Children[stkObjectName];
        var group = (IAgDataProviderGroup) stkEntityObject.DataProviders["Heading"];
        var provider = (IAgDataProvider) group.Group["Fixed"];
        IAgDrResult result = ((IAgDataPrvTimeVar) provider).ExecSingle(_stkObjectRootToIsolateForEntities.CurrentTime);
        stkObjectHeadingAndVelocity[0] = (double) result.DataSets[1].GetValues().GetValue(0);
        stkObjectHeadingAndVelocity[1] = (double) result.DataSets[4].GetValues().GetValue(0);
    }

The issue of code redundancy has been anserwed by Restuta

Solution 6:[6]

Use enums with bit flags:

[Flags]
public enum MyFlags
{
    SomeFlag = 0x1, // 001
    OtherFlag = 0x2,// 010
    ThirdFlag = 0x4 // 100
}

var firstObject = MyFlags.SomeFlag;
var secondObject = MyFlags.SomeFlag | MyFlags.OtherFlag;

if(((int)secondObject & MyFlags.SomeFlag) != 0) 
{
    // true
}

if(((int)secondObject & MyFlags.OtherFlag) != 0) 
{
    // true
}

if(((int)firstObject & MyFlags.SomeFlag) != 0) 
{
    // true
}

if(((int)firstObject & MyFlags.OtherFlag) != 0) 
{
    // false
}

This article would be helpful.

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1 Fredrik Mörk
Solution 2 John Gibb
Solution 3 Raine
Solution 4 ChrisF
Solution 5 bic
Solution 6 Victor Haydin

Related Questions