Skip to content

Improved Enum output - #63

Open
ThranMaru wants to merge 2 commits into
kbilsted:masterfrom
ThranMaru:patch-1
Open

Improved Enum output#63
ThranMaru wants to merge 2 commits into
kbilsted:masterfrom
ThranMaru:patch-1

Conversation

@ThranMaru

@ThranMaru ThranMaru commented Feb 15, 2019

Copy link
Copy Markdown

I noticed, that Enum.GetName(Type, object) gives empty string for values that are not defined in the enum. Those can exist in C#. The ways to create them are the Enum.Parse method and explicit conversion from numeric type (EnumType)42. And because this can lead to errors and confusions, it's especially important that they print out.
Furthermore, empty string is even returned for combined values of enum, even though it has the [Flags] attribute (EnumType.Flag1 | EnumType.Flag2), if they're not named (Flag1And2 = Flag1 | Flag2).
Proposed method of using ToString() solves both these issues by printing numeric value for unnamed values (e.g. "42") and even recognizing when the enum is defined with Flags and the value consist of combination of them and providing list (e.g. "Flag1, Flag2").

I noticed, that `Enum.GetName(Type, object)` prints gives empty string for values that are not defined in the enum. Those can exist in C#. The ways to create them are the `Enum.Parse` method and explicit conversion from numeric type `(EnumType)42`. And because this can lead to errors and confusions, it's especially important that they print out.
Furthermore, empty string is even returned for combined values of enum, even though it has the [Flags] attribute (`EnumType.Flag1 | EnumType.Flag2`), if they're not named (`Flag1And2 = Flag1 | Flag2').
Proposed method of using `ToString()` solves both these issues by printing numeric value for unnamed values (e.g. `"42"`) and even recognizing when the enum is defined with `Flags` and the value consist of combination of them and providing list (e.g. `"Flag1, Flag2"`).
@kbilsted

Copy link
Copy Markdown
Owner

Hi @ThranMaru

Many thanks for the PR. Can I ask you add some unit tests so we ensure we have covered all corner cases etc?

cheers

@DannyTxavace

Copy link
Copy Markdown

Good catch, and thanks for digging into this! You're right that Enum.GetName silently returns null for values that are not exact named enum members, which is precisely the case where we'd want visibility rather than silence.

The fix is simple: use ToString() instead.

public string Convert(object source)
{
    return source.ToString();
}

Why this covers both cases:

  • Undefined values (e.g. (SomeEnum)42 via an explicit cast, or Enum.Parse with a numeric string): GetName returns null because there is no matching member. ToString() falls back to the numeric value, so you get "42" instead of an empty result.
  • Unnamed flag combinations (e.g. Flag1 | Flag2 when there is no Flag1And2 = Flag1 | Flag2 member defined): if the enum is marked with [Flags], ToString() decomposes the value into named members and returns "Flag1, Flag2". Without [Flags], it returns the numeric value, which is still more informative than an empty result.

One thing worth mentioning: the original snippet had a return Enum.GetName(...) on one line and return source.ToString() on the next. That means the ToString() call was unreachable dead code and never executed, so it may be worth checking for the same pattern elsewhere in the file.

Also, source.ToString() will throw if source is null. GetName was not null-safe either, but if Convert can realistically receive null, it may be worth adding an explicit guard while touching this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants