CRM 2011 Plugin - Does using early bound entities for attribute names cause memory issues?

Question

In my plugin code, I use early bound entities (generated via the crmsvcutil). Within my code, I am using MemberExpression to retrieve the name of the property. For instance, if I want the full name of the user who initiated the plugin I do the following

SystemUser pluginExecutedBy = new SystemUser();
pluginExecutedBy = Common.RetrieveEntity(service
                                        , SystemUser.EntityLogicalName
                                        , new ColumnSet(new string[] {Common.GetPropertyName(() => pluginExecutedBy.FullName)})
                                        , localContext.PluginExecutionContext.InitiatingUserId).ToEntity<SystemUser>();

The code for GetPropertyName is as follows

    public static string GetPropertyName<T>(Expression<Func<T>> expression)
    {
        MemberExpression body = (MemberExpression)expression.Body;
        return body.Member.Name.ToLower();
    }

The code for RetrieveEntity is as follows

public static Entity RetrieveEntity(IOrganizationService xrmService, string entityName, ColumnSet columns, Guid entityId)
        {
            return (Entity)xrmService.Retrieve(entityName, entityId, columns);
        }

My solution architect's comments:

Instead of writing the code like above, why not write it like this (hardcoding the name of the field - or use a struct).

SystemUser pluginExecutedBy = null;
pluginExecutedBy = Common.RetrieveEntity(service
                                        , SystemUser.EntityLogicalName
                                        , new ColumnSet(new string[] {"fullname"})
                                        , localContext.PluginExecutionContext.InitiatingUserId).ToEntity<SystemUser>();

Reason:

Your code unnecessarily creates an object before it requires it (as you instantiate the object with the new keyword before the RetrieveEntity in order to use it with my GetProperty method) which is bad programming practice. In my code, I have never used the new keyword, but merely casting it and casting does not create a new object. Now, I am no expert in C# or .NET, but I like to read and try out different things. So, I looked up the Microsoft.Xrm.Sdk.dll and found that ToEntity within Sdk, actually did create a new Entity using the keyword new.
If the Common.Retrieve returns null, your code has unnecessarily allocated memory which will cause performance issues whereas mine would not? A managed language like C# "manages the memory" for me, does it not?

Question

Is my code badly written? If so, why? If it is better - why is it? (I believe it is a lot more cleaner and even if a field name changes as long as as the early bound class file is regenerated, I do not have to re-write any code)
I agree that cast does not create a new object, but does my code unnecessarily create objects?
Is there a better way (a completely different third way) to write the code?

Note: I suggested using the GetPropertyName because, he was hard-coding attribute names all over his code and so in a different project which did not use early bound entities I used structs for attribute names - something like below. I did this 3 weeks into my new job with CRM 2011 but later on discovered the magic of MemberExpression. He was writing a massive cs file for each of the entity that he was using in his plugin and I told him he did not have to do any of this as he could just use my GetPropertyName method in his plugin and get all the fields required and that prompted this code review comments. Normally he does not do a code review.

public class ClientName
{
    public struct EntityNameA
    {
        public const string LogicalName = "new_EntityNameA";
        public struct Attributes
        {
            public const string Name = "new_name";
            public const string Status = "new_status";
        }
    }
}

PS: Or is the question / time spent analyzing just not worth it?

It is never not worth asking a question and trying to become better at what you're doing. Also, criticizing your maybe slightly inefficient but not wrong solution to get at the attribute name in which you invested some creative thought while approving of the use of a static class and method doesn't let me think too highly of this "architect". — TeaDrivenDev
@GCATNM - You and your double negatives! ;-) It took me a good 30 seconds to read the "never not worth" - the eyes read it, the brain interpreted it as "It is never worth ..." and I was half-inclined to click on Delete. Time to go and rest as it is 30 minutes to midnight! — Kanini

Guido Preite Guido Preite · Accepted Answer · 2013-04-29T22:10:26

Early Bound, Late Bound, MemberExpression, bla bla bla :)

I can understand the "philosophy", but looking at your code a giant alarm popup in my head:

public static Entity RetrieveEntity(IOrganizationService xrmService, string entityName, ColumnSet columns, Guid entityId)
        {
            return (Entity)xrmService.Retrieve(entityName, entityId, columns);
        }

the Retrieve throws an exception if the record is not found.

About the other things, the GetPropertyName is ok, but are always choices, for example I try to use always late bound in plugins, maybe in a project I prefer to use early bound, often there is more than one way to resolve a problem.

Happy crm coding!

CRM 2011 Plugin - Does using early bound entities for attribute names cause memory issues?

2 Answers