VrCapabilities valueForString doesn't follow project design

[mikeburke106]

VrCapabilities valueForString method doesn't follow the same design as the rest of the enums in the project.

Most enums look like this:

public static PrerecordedSpeech valueForString(String value) {
    return valueOf(value);
}

But, this class looks like this:

public static VrCapabilities valueForString(String value) {
    if (value.toUpperCase().equals(VrCapabilities.Text.toString().toUpperCase()))
    {
        return VrCapabilities.Text;
    }
    return null;
}

Not sure why we're ignoring case on this one, I don't believe that is correct. Likewise, if another value is added to this enum, additional work would need to be done to add any new values to this method, which is bad practice.

Recommend refactoring this method to follow the same design as other enums.

[mrapitis]

This change was initially implemented for legacy compatibility. Looks like the original request suggested using the toUpperCase method that is in question. Details can be found here: https://bitbucket.org/fmcalt/al_android_dev/issue/29/protect-vrcapabilities-enum-value-for

[mikeburke106]

Ok, this needs to be added to list of known hacks and corner cases.

Ideally, this should use equalsIgnoreCase instead of calling toUpper on both strings. There is still the issue that if any other enum values are added to this enum, additional logic will have to be implemented as well because this method will return null if the new enum string is passed into this method. So, instead of just returning null, we should probably return valueOf(value).

[mikeburke106]

Not everyone can see this private bitbucket page, so here's the information:

Stefan Bankowski created an issue 2013-08-13

The XML specification had been updated to make all the enumerated values consistent. In doing so, the vrCapabilities (currently only one) value of "Text" is now the proper "TEXT".

The proxy needs to handle both cases and treat them the same. The suggested method is a "toUpper()" on the enumerated plain-text value.

I also notice that values of TExt, TeXt, etc will be accepted using this implementation. Not sure if that's acceptable or not, but just wanted to make a note of it.

[mikeburke106]

Stefan's comment also brings up a few more questions. If the XML spec was updated to make all enumerated values consistent, does that mean all enumerations should be all caps?

For example, ImageFieldName and TextFieldName aren't consistent with the other enum values, so is the spec still wrong in that regard, or is the code wrong in this case?

I think, technically, since the enum value was changed to all caps, the enum value in VrCapabilities should also be redefined as all caps to prevent confusion.