Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a extension method like following that takes Anonymous type objects and returns it after converting into ExpandoObject which falls under Dynamic I guess.

public static dynamic ToExpando1(this object AnonymousObject)
{
    IDictionary<string, Object> expando = new ExpandoObject();    
    foreach (var property in AnonymousObject.GetType().GetProperties())
    {
        expando[property.Name] = property.GetValue(AnonymousObject);
    }    
    return (dynamic)expando;
}

The state of the collection in View:

ExpandoObject

When KeyValuePair or Dictionary is returned:

KeyValuePair

If ExpandoObject is the type returned than I get strong typing features. Otherwise, I cannot get the properties values using dot operator like item.Name

How can I make this code more compact with Linq? Because I am not being able to cast if I write Linq and not getting the desired result.

share|improve this question

closed as off-topic by Malachi, syb0rg, p.s.w.g, Jeff Vanzella, BenVlodgi Apr 15 at 21:05

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. Such questions may be suitable for Stack Overflow or Programmers. After the question has been edited to contain working code, we will consider reopening it." – Malachi, syb0rg, p.s.w.g, Jeff Vanzella, BenVlodgi
If this question can be reworded to fit the rules in the help center, please edit the question.

1  
OK, a couple of questions... 1. Why do you want to use Linq (Language Integrated Query) here? What are you trying to do, simply replace the foreach? 2. Why use this method at all? why not just use var dynamicObject = (dynamic)AnonymousObject;? –  Trevor Pilley Apr 15 at 11:44
    
@TrevorPilley Thanks for replying. I want to use linq so that I can make the code more compact and replace foreach and use of temporary variables if possible. I am using the method to convert anonymous objects to ExapndoObject so that they can be accessed in Views with . operators on properties and just var dynamicObject = (dynamic)AnonymousObject; will not work. –  lbrahim Apr 15 at 11:57
2  
@Md.Ibrahim It will work - create a console app with the following code: var thing = new { Name = "Bob" }; var dynamicThing = (dynamic)thing; Console.WriteLine(dynamicThing.Name); –  Trevor Pilley Apr 15 at 12:33

2 Answers 2

I think this is abusing anonymous types, dynamic and extension methods.

Anonymous Types

These things are very useful in LINQ; they allow you to define a type on-the-fly to hold groupings (with the GroupBy method), for example. Anonymous types should only live within a method, and not leak outside of it - this means returning an anonymous type is bad, see this Jon Skeet answer on Stack Overflow.

Dynamic Types

Don't get me wrong: dynamic is awesome. But whenever I encounter it, I do everything I can to "contain" it and get back into compile-time strongly-typed code. The reason is that whatever it touches becomes dynamic as well, making more and more code that's not resolvable at compile-time. Use with caution.

Extension Methods

You're extending System.Object, which means literally anything in the Wonderful World of .NET can be passed to your method, and now has a ToExpando1 method: extending anonymous types just isn't right, these things are meant to be temporary, on-the-fly objects. If you need first-class citizens, create a new class.


Note: this is all based on not knowing how this extension method is being used! Based on comments however:

I am using the method to convert anonymous objects to ExpandoObject so that they can be accessed in Views [...]

This might make sense. I don't do enough Web development to be a reference on the matter, but it seems to me that the model accessed by a view would be much better off as a full-fledged class, instead of just a bag of properties glued together into an anonymous/dynamic type.


Nitpicks/Review

I don't like the naming style:

public static dynamic ToExpando1(this object AnonymousObject)

Why there is a 1 there isn't clear to me (looking at the post's edit history is instructive though - thanks for making that edit!). Also AnonymousObject being a local-scope parameter, should be named anonymousObject, and having Object in an object's name just doesn't feel right. How about that instead?

public static dynamic ToExpando(this object that)

In the method's body, you're declaring expando as an IDictionary<string, Object>:

IDictionary<string, Object> expando = new ExpandoObject(); 

It's not clear why System.Object isn't referred to as its language alias object (as it is in the method's signature), but since you're casting expando to dynamic before returning it, I don't see why you can't be just declaring it with var:

var expando = new ExpandoObject(); 

The method uses System.Reflection types correctly, I like that.


Why your original LINQ code didn't work

As @Sandeep has suggested, the correct usage of LINQ is to project a selection using the Select method, like this:

anonymousObject.GetType()
               .GetProperties()
               .Select(p => new KeyValuePair<string, object>(p.Name, p.GetValue(anonymousObject));

What your original code does, had side-effects (assigning a value into an array index) - LINQ stands for Language-INtegrated Query, querying objects isn't something that should have side-effects, hence assigning expando[property.Name] can't work in a Select call.

share|improve this answer
    
@MatsMug Thanks for your detailed review and I do agree with you on all of it but I think it'd be helpful in some parts of our program. Btw, your linq query is not working. I will edit my question to include screenshots of the type returned and gotten in the View. –  lbrahim Apr 15 at 14:42
    
Sorry I didn't test any of that code, I based the LINQ on @Sandeep's code - but you shouldn't be using LINQ here anyway ;) –  Mat's Mug Apr 15 at 14:58

This should help. However, I am still not able to appreciate the importance of converting the code to a linq query:

IDictionary<string, Object> expando = new ExpandoObject();
IEnumerable<PropertyInfo> properties = AnonymousObject.GetType().GetProperties();

var test = from property in properties
            select new KeyValuePair<string, object>(property.Name, property.GetValue(AnonymousObject));

return expando.Concat(test);
share|improve this answer
    
Concat returns IEnumerable. –  abuzittin gillifirca Apr 15 at 12:40
    
@Sandeep If I am able to use linq then the same code can be made compact and I can also prevent from using all the temporary variables like expando, properties, test and make it more efficient. –  lbrahim Apr 15 at 14:47
1  
@Ibrahim Going by the original code, do you really need an Expando object. A Dictionary<string, Object> will do the job. abuzittin You are right. It is a list of KeyValuePair which can be easily converted to a dictionary using .ToDictionary extension method. –  Sandeep Apr 15 at 18:17

Not the answer you're looking for? Browse other questions tagged or ask your own question.