microsoft
[Enum] Add support DescriptionAttribute and DisplayAttribute
If DisplayAttribute is not found, look for DescriptionAttribute. If not found, use value.
@dvoituron and I were just discussing this yesterday. As we can use the DisplayAttribute now without pulling in a dependency on EF, we think it makes sense to use this standard mechanism and phase out the 'description variant' we built in ourselves.
So instead of using this approach you suggest with this PR, I would rather have two separate methods. Besides a new GetDisplayAttribute we still have GetDescription which we obsolete with the right attributes and can remove in a later version. We just need to be smart on how we define the correct overloads.
Once that is in place we can replace all the [Description(...)] ones we have in our code. Makes sense?
We can pass all our DescriptionAttribute to DisplayAttribute I don't think anyone externally has ever read these properties. Two methods is ok
but users have classes and enumerators with DisplayAttribute/DescriptionAttribute/DisplayNameAttribute. If we want to make it easier for them, shouldn't we read them all?
but users have classes and enumerators with DisplayAttribute/DescriptionAttribute/DisplayNameAttribute. If we want to make it easier for them, shouldn't we read them all?
I don't think so, we cannot manage all attributes. We could allow a configuration section (in the Global Config) or another way. For performance and code maintenance reasons, we need to select 1 attribute.
but users have classes and enumerators with DisplayAttribute/DescriptionAttribute/DisplayNameAttribute. If we want to make it easier for them, shouldn't we read them all?
I don't think so, we cannot manage all attributes. We could allow a configuration section (in the Global Config) or another way. For performance and code maintenance reasons, we need to select 1 attribute.
Exactly. And because we made GetDescription public, we must think about somebody using that in their code base so we can't just remove it.
I like the configuration setting idea.
Oh, and btw, Denis and I were talking about this as a part of vNext (major) so not necessarily for the next point release. But if we can manage to cater to all scenarios we can certainly do it sooner.
But to give it some thought a not rush through it, I don't want to finish this for our (imminent) v4.9 release already.
Again, I don't think this kind of modification is relevant right now.
Besides, we can't change all the Description attributes to Display without causing major breaking changes for all library users. It's therefore conceivable for version 5, but not before.
Just to understand, what problem do you have that you want to solve with an RP like this?
Code cleanup and simplification. I don't think users read the [Description] attributes. Definitely for the new [Display] usage.
Of course, developers can (and do) work with the public attributes of enumerations.
On the other hand, I don't think it's a good idea to modify ALL attributes in Display.
- Display: Gets or sets a value that is used to display a description in the UI.
- Description: Specifies a description for a property or event.
In other words, all enumerations converted to "string" for use in HTML must remain in Description.