1

I have an enum with a custom attribute marking the items:

enum MyColourEnum {        
    [RenderAs("'#ff0000'")]
    Red,

    [RenderAs("'#00ff00'")]
    Green
}

Then I create an expression tree that uses the enum:

Expression<Func<Environment,bool>> expr = _ => _.Colour == MyColourEnum.Red;

I then parse the expression tree and translate it into a string representation of the expression. The resulting string that I want is:

"environment.colour == '#ff0000'"

The problem I have is that the enum value gets turned into a constant within the lambda, so when looking at the expression tree, it sees the constant value 0 instead of an expression reading the Red item of the enum.

I want to use the custom attribute to identify the enum as a special case, and replace it's value with the one attached to the attribute, but I can't because all I can see is the constant 0 value.

How can I get the enum used to create the constant within the expression tree?

If you can't, then how else can I do something similar?

BG100
  • 4,481
  • 2
  • 37
  • 64
  • BTW, the convention is to call a lambda parameter `_` if you're *not* going to use it. – svick Sep 30 '14 at 00:48
  • @svick: is this documented anywhere? – BG100 Sep 30 '14 at 11:31
  • Not really documented, since I believe it's not an official convention, but you can have a look at for example http://stackoverflow.com/q/2778362/41071. – svick Sep 30 '14 at 12:38

2 Answers2

2

For the particular example, following code works.

Expression<Func<Environment,bool>> expr = _ => _.Colour == MyColourEnum.Red;
BinaryExpression binaryExpression = (BinaryExpression)expr.Body;

var convert = (UnaryExpression)binaryExpression.Left;
var propertyExpression = (MemberExpression)convert.Operand;
var property = (PropertyInfo)propertyExpression.Member;

Enum enumValue = (Enum)Enum.ToObject(property.PropertyType, ((ConstantExpression)binaryExpression.Right).Value); //
FieldInfo fi = property.PropertyType.GetField(enumValue.ToString());
var renderAs = fi.GetCustomAttribute<RenderAsAttribute>();

if (renderAs != null)
{
    String color = renderAs.Color;

    Console.WriteLine("{0}.{1} == {2}", property.DeclaringType.Name, property.Name, color);
}

For now, I hardcoded == operator, if you want to make it dynamic you need to inspect binaryExpression.NodeType property.

Note: This won't work properly when your enum have duplicate values. (I.e) More than one enum field with same value. Not that problem with above code, Enum.ToObject is broken when it finds a duplicate. In fact most of the methods in enum won't work properly when you have duplicates.

Sriram Sakthivel
  • 72,067
  • 7
  • 111
  • 189
  • This is only going to work for that one example case, it doesn't ensure that any arbitrary expression will use this attribute to render that enum when converting it to a string – Servy Sep 29 '14 at 14:05
  • @Servy I've already said that in my answer. Also OP didn't asked for generic solution(Unless am overlooking something). – Sriram Sakthivel Sep 29 '14 at 14:08
  • What makes you think he won't ever have any other types of expressions that he'll be using? He asked how to determine how this enum is rendered from expression trees. This doesn't do that. – Servy Sep 29 '14 at 14:13
  • @Servy That brings a question how can you know what are all the kinds of expressions he'll be using? Well, If op need a generic solution and needs to cover some other kinds of expressions let him drop a comment. I'll be happy to revise my answer. – Sriram Sakthivel Sep 29 '14 at 14:17
  • If the OP will never ever have any other types of expression that he'll be working with he might as well just using a string literal from the start and not even bother touching expressions in the first place. The only reason to bother using an `Expression` in the first place is because you *don't* know exactly what the contents will be. – Servy Sep 29 '14 at 14:20
  • @SriramSakthivel: This is perfect, exactly what I'm looking for. I didn't think about examining the LHS to see if the Type as an enum. – BG100 Sep 29 '14 at 14:23
  • @Servy: This is a special case, I can put code around this to handle other expressions. This perfectly answers my question. – BG100 Sep 29 '14 at 14:25
  • @BG100 If you're going to special case every single possible expression then *you shouldn't be using expressions in the first place* as they're only ever making your code more complex, not any simpler. – Servy Sep 29 '14 at 14:26
  • @Servy Assuming you have downvoted. Your downvote is not fair. You make too many assumptions. Expression can be whatever expression, say `MethodCallExpression`, `Unary`, `Binary` and so forth. So you want to write tons of code to cover all? Makes no sense for me. – Sriram Sakthivel Sep 29 '14 at 14:27
  • @Servy: Without knowing more context around exactly what I'm doing with the expression, then I can't see how you can say that... without going in to too much detail, I'm trying to render C# expressions as EcmaScript, and this answer works perfectly to handle one of a handful of special cases. – BG100 Sep 29 '14 at 14:30
  • @SriramSakthivel The fact that you're saying that only goes to show that you don't understand enough about how to work with expressions to provide a good answer. You shouldn't be trying to special case every single possible type of expression, rather you should be writing a generalized solution that doesn't have *any* special cases, and that is never dependent on the structure of the expression. If you only ever have this exact expression then you shouldn't be going through the steps of using an `Expression` in the first place, and you should just build the string from scratch. – Servy Sep 29 '14 at 14:31
  • @Servy Handling the special cases alone [sometimes makes sense](http://stackoverflow.com/questions/671968/retrieving-property-name-from-lambda-expression). – Sriram Sakthivel Sep 29 '14 at 14:37
  • @SriramSakthivel No, it doesn't. If this exact expression is the only type of expression that he ever has then he shouldn't be using expressions in the first place. If the entire structure of the expression is known then the entirety of your expression manipulation code is superfluous and is wasted effort, needlessly increasing the complexity of the code. – Servy Sep 29 '14 at 14:40
  • @Servy: What are you talking about? The structure of the expression is NOT known, its not fixed, and can be set to anything, hence why I'm using expressions! – BG100 Sep 29 '14 at 14:58
  • @BG100 If that's the case then this code won't work. It's highly dependent on the exact structure of this code. It requires that the expression be an equality operator in which the left hand side is a property access of the lambda's parameter and the right hand side is a constant expression of this one enum's type. If there is any variation from that then this code won't work. If the enum is closed over, if the left hand side is a property of a property, if the constant is on the left hand side, if the constant is replaced with a method call/property/static field, etc. this code won't work. – Servy Sep 29 '14 at 15:14
  • @Servy: As noted in this answer, the equality operator was hardcoded for brevity, but that wasn't part of my question as I can easily handle that without help. I'd already coded it to handle any operator anyway. This answers my question perfectly, I've implemented it in my code, and my code works... and I haven't done some crazy re-inventing the wheel thing with millions of special cases. – BG100 Sep 29 '14 at 15:50
  • @BG100 I think you didn't get what servy says. He says if your expression is something like this `MyColourEnum.Red == _.Colour` or `_.Colour == GetMyEnum()` or something like that, above code won't work and that's true. I've explicitly mentioned it in my answer in very first line *For the particular example,following code works.* In order for this code to work, you need to have a expression with format `someclass.SomeProperty SomeOperator SomeConstant` – Sriram Sakthivel Sep 29 '14 at 16:02
  • 1
    @SriramSakthivel: Ok, yeah I get that... I'm aware of the limitations. My question was only specifically asking about how to access a custom attribute attached to an enum from within an expression, which you answered perfectly :) and I'm fully aware that its checking for the type on the LHS of the operator, and the expressions are limited to that. I'm just annoyed that your answer was downvoted! – BG100 Sep 29 '14 at 16:16
  • @BG100 No, expressions are *not* limited to that. This answer is limited to that, but it's not a limitation of the tooling. That's what makes it a low quality answer. This is not the proper way to use expressions. If you're writing a method that accepts an expression and you require the expression to be of such an exact format then something is very fundamentally wrong. – Servy Sep 29 '14 at 16:47
  • @Servy If an answer which answers OP's specific question does that makes it *low quality answer*? Don't be ridiculous. That means you have poor understanding of what is a *low quality answer*. Saying fundamentally wrong is not helpful in anyway. How about trying to fix it? Ask for more details and write your own answer? I'll be happy to upvote if that answer deserves it! – Sriram Sakthivel Sep 29 '14 at 17:58
  • @Servy I don't recall I've ever said this is a good answer. I said this answer solves the specific problem and that's true. Do you downvote all the answers which are not good(but an answer)? Go hunt for it, there are millions available. You'll not have enough reputation to downvote them all. Your downvote makes me sad when I really help for OP and that earns a downvote. – Sriram Sakthivel Sep 29 '14 at 18:51
  • @SriramSakthivel Yes, I downvote bad answers to questions, even if they are answers. Why *wouldn't* you downvote a bad answer that you came across? The fact that anyone would consider it appropriate to *upvote* a very bad answer is just *shocking* to me. You're quite right that there are a lot of very bad answers on this site. I do what I can to downvote them when I come across them. Apparently you'd rather be a part of the problem than a part of the solution. That's a shame. – Servy Sep 29 '14 at 18:54
  • @Servy I do downvote bad answers, even if they are answers. but I really don't understand how this is a bad answer. IMO this answer is neither good nor bad. It is just a solution to a specific problem. I'll really appreciate your downvote if you help OP by redirect him to the *pit of success*, I'll also be benefited if you can tell me something new. Saying your design is wrong is no way helpful. Say what we have to do instead? – Sriram Sakthivel Sep 29 '14 at 19:03
  • @SriramSakthivel I've told you exactly why it's a bad solution. It's highly dependent on the *exact* format of the expressions it can contain, rather than writing a solution that's independent of the structure of the expression. The whole point of using expressions is to be able to support arbitrary C# code. – Servy Sep 29 '14 at 19:11
  • 2
    @Servy: If I'm not mistaken, Microsoft do something very similar to what I'm doing with Linq to SQL: They take an expression tree, and walk the tree to translate the expression into a SQL expression. SQL does not support every possible expression, as apparent when you get the `does not recognize the xxx method, and this method cannot be translated into a store expression` exception. What I'm doing is no different. Are you saying that Microsoft do it the wrong way also? The only difference is that I'm translating the expression into EcmaScript rather than SQL. – BG100 Sep 29 '14 at 21:48
  • @BG100 The LINQ implementations don't support only one exact expression that allows for literally no variance whatsoever. The LINQ query providers account for many thousands of different permutations, well beyond the realm of reasonable possibilities to use without using expressions. If you're never ever going to allow for any possibilities beyond just this one operation then you're gaining *nothing* from the use of expressions. You're spending a tone of time and effort doing something that could be done in 1/5th the code (and much simpler code) without using expressions. – Servy Sep 30 '14 at 14:01
  • @Servy: I've never said I'm never ever going to allow for anything beyond one operation... If I did I of course I wouldn't be using expression trees!! I'm building something to translate arbitary C# lambda expressions into equivalent EcmaScript that supports all of the finite types of expressions that I need it to, but not all infinite possiblities of every expression that there ever was... you clearly don't understand what I'm trying to do here. – BG100 Sep 30 '14 at 14:07
  • @BG100 Well this code can't handle *anything* else. If this is "the perfect answer to your question" then clearly that's the situation you're in. If you're actually dealing with a wider range of possibilities then *this isn't the right solution for your problem* and you'll want to be using a more generalized solution that isn't so highly dependent on the exact structure of the expression. Especially considering that [a generalized solution that will work with any possible expression is in fact far simpler and easier](http://stackoverflow.com/a/26101788/1159478). – Servy Sep 30 '14 at 14:15
  • @Servy: This answer doesn't show my entire code! I have already written the (generalised) code that handles the vast majority of the expressions that I need it to, as I said this snippet just gets me past a rare special case that I need it to handle that I've now embedded into the rest of my code surrounded by an `if` that detects the special case. My code isn't a huge list of hardcoded fixed expressions, it builds the EcmaScript expression up piece by piece from the expression tree. – BG100 Sep 30 '14 at 14:23
  • @BG100 And then the user of your code is going to reverse the order of the operands, or use a non-constant enum value, or use a more complex operation to get a value from the parameter, or any number of other trivial changes that will end up crashing your code because you felt the need to apply arbitrary restrictions on the *exact* formatting of this operation despite the fact that the other variations have entirely sensible translations by your query provider. And all of that so you can use more code and more complex code than a generalized solution. – Servy Sep 30 '14 at 14:27
1

The Enum type is not for that! It's wrong to use custom user attributes with Enum, it is wrond development way. Finally it always becomes some monster which hard to support. You'll better create some abstract class:

public abstract class DescriptedCodeValue
{
    protected DescriptedCodeValue(int id, string description)
    {
        Id = id;
        Description = description;
    }
    public int Id { get; private set; }
    public string Description { get; private set; }

    public static implicit operator int(DescriptedCodeValue val)
    {
        return val.Id;
    }

    public static implicit operator string(DescriptedCodeValue val)
    {
        return val.Description;
    }

    public override string ToString()
    {
        return Description;
    }
}

After that you simply inherit it, for example like this:

public class ColorCode : DescriptedCodeValue
{
    private ColorCode(int id, string description) : base(id, description) { }
    public static ColorCode Red = new ColorCode(1, "#ff0000");
    public static ColorCode Green = new ColorCode(2, "#00ff00");
}

Finaly, it is simply example. You can easily optimize that class to your needs, and the main profit is that you can easily extend functionality wherever you want without serious changes in project.

Jonik
  • 1,208
  • 2
  • 12
  • 20
  • I would say that this is also a correct answer, and will work just as well... but I have chosen to implement SriramSakthivel's answer, so thats why I've marked his answer as correct. If I could have selected two answers then I would have :) – BG100 Sep 29 '14 at 14:27