Anonymous objects: when a named object just won't do



  • I discovered this gem while doing maintenance on a system which supports "ad hoc" users as a temporary mechanism: someone who has authority to assign a role but not to create a user can supply the name and e-mail of the person whom they want to perform role quux for a foo, and the admins can later promote the ad hoc user to a real user account. However, that context is unnecessary to appreciate the WTFery.

    public static List<AdHocListHelper> GetAdHocUsersNotInSystem()
    {
    	var foos = DB.RetrieveAll<Foo>();
    
    	var quuxsInvited = foos
    		.Where(foo => foo.QuuxEmail != null && foo.QuuxName != null)
    		.Select(foo => new
    			{
    				FooName = foo.Name,
    				BarTitle = foo.Bar.Title,
    				Name = foo.QuuxName,
    				Email = foo.QuuxEmail,
    				Organisation = foo.Organisation.Name,
    				FooId = foo.Id
    			})
    		.ToList();
    
    	var quuxsNotInSystem = quuxsInvited
    		.Where(quuxInvited => UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null)
    		.Cast<object>()
    		.ToList();
    
    	return quuxsNotInSystem.Select(quuxNotInSystem => new AdHocListHelper()
    		{
    			Name = quuxNotInSystem.GetType()
    				.GetProperty("Name")
    				.GetValue(quuxNotInSystem, null)
    				.ToString(),
    			Email = quuxNotInSystem.GetType()
    				.GetProperty("Email")
    				.GetValue(quuxNotInSystem, null)
    				.ToString(),
    			Organisation = quuxNotInSystem.GetType()
    				.GetProperty("Organisation")
    				.GetValue(quuxNotInSystem, null)
    				.ToString(),
    			FooName = quuxNotInSystem.GetType()
    				.GetProperty("FooName")
    				.GetValue(quuxNotInSystem, null)
    				.ToString(),
    			BarTitle = quuxNotInSystem.GetType()
    				.GetProperty("BarTitle")
    				.GetValue(quuxNotInSystem, null)
    				.ToString(),
    			FooId = Convert.ToInt32(quuxNotInSystem.GetType()
    				.GetProperty("FooId")
    				.GetValue(quuxNotInSystem, null))
    		}).ToList();
    }

    For people who aren't familiar with C# v3 or above, the second statement is creating a list of "anonymous" objects - that is to say, the compiler will create a type with precisely the members specified, but because the type doesn't have a name it's restricted to a small scope. The second statement filters that list, and the third statement uses reflection to get at the properties of the anonymous object in order to build another object, this time of the type AdHocListHelper. Also note that the keyword var means that the compiler will infer the type from the expression on the right-hand side.

    WTF #1: the person who wrote this didn't see six almost identical statements and think "Perhaps I can factor out this behaviour".

    WTF #2: the person who wrote this didn't see six uses of reflection to access a members of an object which is created in the same method and think "Perhaps this isn't the best way of doing it".

    WTF #3: if we remove the line .Cast<object>() then the compiler will happily infer a type and allow access to the properties of the anonymous objects without reflection.

    WTF #4: looking at the mapping from anonymous object to AdHocListHelper, there seems to be no reason for the anonymous type to exist in the first place. The only reason it might be preferable would be if the constructor of AdHocListHelper was sufficiently WTFy that it wasn't safe to create instances which we're not going to return, and (fortunately) it isn't.



  •  I understood all this and I wholeheartedly agree with you on the WTFery.


  • ♿ (Parody)

    @pjt33 said:

    WTF #3: if we remove the line .Cast<object>() then the compiler will happily infer a type and allow access to the properties of the anonymous objects without reflection.

    And they know that they can do this, because they did it in the line right above .Cast<object>().

    @pjt33 said:

    WTF #4: looking at the mapping from anonymous object to AdHocListHelper, there seems to be no reason for the anonymous type to exist in the first place. The only reason it might be preferable would be if the constructor of AdHocListHelper was sufficiently WTFy that it wasn't safe to create instances which we're not going to return, and (fortunately) it isn't.

    Is there no way to incorporate UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null into the original query / expression / filter (or whatever you call this)? Seems like you could compress this all into a single statement at that point and just create the appropriate AdHocListHelpers to begin with. Something like:

    return foos.Where(foo => foo.QuuxEmail != null && foo.QuuxName != null 
    		&& UtilitiesHelper.GetUserByEmail( foo.QuuxEmail ) == null)
    	.Select(foo => newAdHocListHelper()
    		{
    			Name = foo.Name,
    			BarTitle = foo.Bar.Title,
    			Name = foo.QuuxName,
    			Email = foo.QuuxEmail,
    			Organisation = foo.Organisation.Name,
    			FooId = foo.Id
    		})
    	.ToList();
    


  • @boomzilla said:

    Is there no way to incorporate UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null into the original query / expression / filter (or whatever you call this)?

    Unfortunately not. Because you're going against a DB, the linq to sql converts your where clause into a sql statement, which is passes to the sql server to execute. If you add in a call to a function, that gets passed along to the sql server too, which obviously can't execute your code.

    This has bitten me more often than I care to admit; and that's why the query needs to be done in stages.



  • @boomzilla said:

    Is there no way to incorporate UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null into the original query / expression / filter (or whatever you call this)? Seems like you could compress this all into a single statement at that point and just create the appropriate AdHocListHelpers to begin with.


    Yes. I actually refactored it slightly differently in order to share a lot of code with another method which used the opposite filter.



  • @DrPepper said:

    @boomzilla said:
    Is there no way to incorporate UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null into the original query / expression / filter (or whatever you call this)?

    Unfortunately not. Because you're going against a DB, the linq to sql converts your where clause into a sql statement, which is passes to the sql server to execute. If you add in a call to a function, that gets passed along to the sql server too, which obviously can't execute your code.

    This has bitten me more often than I care to admit; and that's why the query needs to be done in stages.

    Hence the AsEnumerable() method. At first glance, the AsEnumerable method appears to be completely useless, but it can be used to "step out of" the SQL context:

    public static List<AdHocListHelper> GetAdHocUsersNotInSystem()
    {
    	return DB.RetrieveAll<Foo>()
    		.Where(foo => foo.QuuxEmail != null && foo.QuuxName != null)
    		.Select(foo => new AdHocListHelper
    			{
    				FooName = foo.Name,
    				BarTitle = foo.Bar.Title,
    				Name = foo.QuuxName,
    				Email = foo.QuuxEmail,
    				Organisation = foo.Organisation.Name,
    				FooId = foo.Id
    			})
    		.AsEnumerable()
    		.Where(quuxInvited => UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null)
    		.ToList();
    }
    

  • ♿ (Parody)

    @DrPepper said:

    @boomzilla said:
    Is there no way to incorporate UtilitiesHelper.GetUserByEmail(quuxInvited.Email) == null into the original query / expression / filter (or whatever you call this)?

    Unfortunately not. Because you're going against a DB, the linq to sql converts your where clause into a sql statement, which is passes to the sql server to execute. If you add in a call to a function, that gets passed along to the sql server too, which obviously can't execute your code.

    This has bitten me more often than I care to admit; and that's why the query needs to be done in stages.

    OK, but your users should be part of the DB, no? Even if they're in an LDAP, you should be able to make your DB talk to the LDAP so you can query for them. Or do whatever that other function is doing. Extra round trips seem like a waste of time, anyways (not we have any idea how performance critical this is, of course).



  • @boomzilla said:

    OK, but your users should be part of the DB, no?

    They're in a different DB. Don't get me started on the "single sign-on" system this project uses...



  • @StephenCleary said:

    Hence the AsEnumerable() method. At first glance, the AsEnumerable method appears to be completely useless, but it can be used to "step out of" the SQL context:

    Good catch; the AsEnumerable() realizes the list; which is then an object in memory; and can be further processed. This is essentially what the OP is doing by using ToList(), but with AsEnumerable() there is no need for an intermediate object.


  • BINNED

    @pjt33 said:

    WTF #2: the person who wrote this didn't see six uses of reflection to access a members of an object which is created in the same method and think "Perhaps this isn't the best way of doing it".
     

    I've never used reflection myself. The only time I read about it it's on TDWTF.
    I've got the feeling that reflection is never the best way of doing it and maybe shouldn't even exist in the first place?!



  • @topspin said:

    I've got the feeling that reflection is never the best way of doing it and maybe shouldn't even exist in the first place?!

    Alas, it has to exist for a few (very few) features.


  • Discourse touched me in a no-no place

    @topspin said:

    I've never used reflection myself. The only time I read about it it's on TDWTF.
    I've got the feeling that reflection is never the best way of doing it and maybe shouldn't even exist in the first place?!
    Used well, it lets you do awesome stuff with minimal code. In the hands of morons… well, it keeps us entertained here at least.



  • @topspin said:

    I've never used reflection myself. The only time I read about it it's on TDWTF.
    I've got the feeling that reflection is never the best way of doing it and maybe shouldn't even exist in the first place?!


    It's absolutely fundamental to Microsoft's approach to RAD. Note that in making the preceding statement I am not disagreeing with you.



  • @topspin said:

    I've got the feeling that reflection is never the best way of doing it and maybe shouldn't even exist in the first place?!

    95% of the time, Reflection is the wrong way to do something. The other 5% of the time, it's the only way. One of the tricks to being a good developer is knowing when that 5% is.



  • @topspin said:

    I've got the feeling that reflection is never the best way of doing it and maybe shouldn't even exist in the first place?!
     

    To quote SunOracle:

    @Oracle said:

    If it is possible to perform an operation without using reflection, then it is preferable to avoid using it.

     



  • @Cassidy said:

    To quote SunOracle:

    @Oracle said:

    If it is possible to perform an operation without using reflection, then it is preferable to avoid using it.

     


    Which must be interpreted correctly. In Java it's always possible to avoid using the reflection API. However, the alternative (which would involve disassembling class files yourself and probably using a custom classloader to inject accessors for the private members) is even worse.


Log in to reply