Skip to content

Instantly share code, notes, and snippets.

@ilkerde
Created February 22, 2011 14:08
Show Gist options
  • Save ilkerde/838709 to your computer and use it in GitHub Desktop.
Save ilkerde/838709 to your computer and use it in GitHub Desktop.
I wonder whether introducing such an extension is considered a smell. For me it's most likely a smell.
using System;
namespace Is.This.A.Smell {
public static class EnumExtensions {
public static string AsString(this Enum e) {
return Enum.GetName(e.GetType(), e);
}
}
}
@forki
Copy link

forki commented Feb 22, 2011

I like it and have a lot of such small extensions.
I think they improve readability and give a more fluent interface.

Just my 2 cents.

Regards Steffen

@thoemmi
Copy link

thoemmi commented Feb 22, 2011

what's wrong with
e.ToString()
?

@ilkerde
Copy link
Author

ilkerde commented Feb 22, 2011

It's about this particular extension. It supports to write the string conversion of the enum value with a more terse syntax. That is, with this extension, one is able to write

string myText = MyStatus.Ready.AsString();

Given that MyStatus is an enumeration. However, when a developer tends to implement such an extension, it most likely means that he has a lot of Enum to string type conversions to deal with. He basically wants to get rid of the clumsy syntax of the Enum GetName() method, which would be:

string myText = Enum.GetName(typeof(MyStatus), MyStatus.Ready);

Which, admittedly, is not nice. Granted, the experienced C# developer now would laugh about this and just ask why the author didn't just use the ToString()-Method:

string myText = MyStatus.Ready.ToString("F");

which essentially is a shortcut to the static GetName() with a few more options. I personally think that the ToString() Method might be not as nice as the extension itself, since it requires you to pass in the formatting option every time (as literal, most likely).

Nonetheless, writing the above extension is a clear indicator for frequent type conversions from a restrictive, finite value-bound type to a much looser type. There's no guaranteed conversion backwards.

Actually, this might be sensible for a few scenarios. However, I argue that for most cases it is simply not needed. A very common scenario of enum to string conversion I've seen in quite a few code-bases is visual representation. That is, in my little example above, it might be that the developer might want to have the "Ready" status displayed somewhere.

As for the visual representation case, this for me is a smell for mixed concerns. The visualization concern needs to be separated from the value semantics, imho.

Another quite common scenario would be a "bridge type conversion". That is, in order to get the value from one domain boundary to another (see bounded contexts of DDD) - or even worse from one technical layer to another (i.e. data -> service) - the developer might perform a bridged type conversion. He would declare the same Enum's in both contexts and (ab)use the lousy string type as a common ground for value assignment. Quite frequent (anti)pattern I've seen in a few "service-oriented" code-bases.

namespace CoolDomain {
public enum MyStatus { None, Ready, Busy, StandBy }
}

namespace ColdDomain {
public enum MyStatus { None, StandBy, Busy, Ready }
}

In such a scenario, frequent type conversion is given. It's a pity here that the enum type is a little beast and might change it's value semantics over time (though it shouldn't), thus rendering the nice "loosely-coupled" type conversion to a risky task.

Generally speaking, if faced with enum conversion, laying back for a while and thinking about the cause of the conversion might reveal more than initially thought. This might not always be the case, however I trapped myself a couple of times where I could avoid the conversion and go with another approach.

So, this particular extension tends to be smelly, imho.

@forki
Copy link

forki commented Feb 22, 2011

Hi Ilker,

I would say if you use it for a map then it's a smell.
But consider https://gist.github.com/837312. You could use your method to replace the string "Abrufauftrag" with it and then you might get better refactoring support.
IMHO the method alone is no smell but within your context it is.

Regards, Steffen

@shishkin
Copy link

So, the smell is not the extension method on an enum, but a string-based enum conversion (mapping) or a view. Agreed.

P.S. You need to try to write less. I consider long comments a smell ;)

@ilkerde
Copy link
Author

ilkerde commented Feb 22, 2011

@forki: granted, makes life easier and hence is a good thing.
@shishkin: thanks. <- better? ;-)

@ilkerde
Copy link
Author

ilkerde commented Feb 22, 2011

@rseso: Very interesting approach. I'd never thought of decorating enum values with metadata.

@shishkin
Copy link

@ilker: Man, that's what I call continuous improvement! ;)

@dun3
Copy link

dun3 commented Feb 23, 2011

Most definitively a smell. But not the extension method itself, but the need for it and the use of Enum in a first place. This seems to no longer be a simple Enum and should most likely be replaced by a proper class(-hierarchy). We found that using an enum is often a premature optimization that will bite you in the end. Especially when decorating enums with attributes ... think very hard before going down that road.

Why did you use an enum in this case?

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