Constructor to transform an Outlook contact into a Dynamics CRM contact

Question

This is my constructor to transform an Outlook contact into a Dynamics CRM contact:

public Contact(ContactItem contactItem)
{
    LogicalName = EntityLogicalName;
    FirstName = contactItem.FirstName;
    LastName = contactItem.LastName;
    //if no last name, use the first name
    if (String.IsNullOrEmpty(LastName))
    {
        LastName = contactItem.FirstName;
        //if no First name, use the Full Name.
        if (String.IsNullOrEmpty(LastName))
        {
            LastName = contactItem.FullName;
            //if no last name or first name, use the company name
            if (String.IsNullOrEmpty(LastName))
            {
                LastName = contactItem.CompanyName;
                //If no last name, no first name and no company name, use the email address.
                if (String.IsNullOrEmpty(LastName))
                {
                    LastName = contactItem.Email1Address;
                    //if no Last name, no first name, no company name, no email address, default to naam niet bekend.
                    if (String.IsNullOrEmpty(LastName))
                    {
                        LastName = @"Naam niet bekend";
                    }
                }
            }
        }
        else
        {
            FirstName = "";
        }
    }

    Address1_City = contactItem.BusinessAddressCity;
    Address1_Country = contactItem.BusinessAddressCountry;
    Address1_Line1 = contactItem.BusinessAddressStreet;
    Address1_PostalCode = contactItem.BusinessAddressPostalCode;
    Telephone1 = contactItem.BusinessTelephoneNumber;
    Telephone2 = contactItem.HomeTelephoneNumber;
    Fax = contactItem.BusinessFaxNumber;
    Address2_Line1 = contactItem.HomeAddressStreet;
    Address2_City = contactItem.HomeAddressCity;
    Address2_PostalCode = contactItem.HomeAddressPostalCode;
    Address2_Country = contactItem.HomeAddressCountry;
    EMailAddress1 = contactItem.Email1Address;
    EMailAddress2 = contactItem.Email2Address;
    EMailAddress3 = contactItem.Email3Address;
}

As you can see, I got 5 nested ifs to determine what I should use for LastName. I don't like how it looks, but the only other option I can see is putting them in sequence instead of nested. However, that would mean that instead of usually having only 1-3 ifs, it would be forced into doing 5 ifs every single time. during tests with 1800 records, it already felt noticeably slower if it got to the literal string.

Is there some really easy way (or some really obscure way) of doing this which I missed?

Answer 1

1. You could create a helper function with a loop:

   public string getFirstNonNullOrEmpty(params string[] names) {
       foreach (string name in names) {
           if (String.IsNullOrEmpty(name)) {
               continue;
           }
           return name;
       }
       return @"Naam niet bekend";
   }
   

   Usage:

   LastName = getFirstNonNullOrEmpty(contactItem.LastName, contactItem.FirstName, 
       contactItem.FullName, contactItem.CompanyName, contactItem.Email1Address);
   

   You might also need this because of the else branch:

   if (String.IsNullOrEmpty(contactItem.LastName)) 
       && !String.IsNullOrEmpty(contactItem.FirstName)) {
       FirstName = "";
   }
   

2. You might want to use IsNullOrWhiteSpace instead of IsNullOrEmpty.

(I'm not too familiar with C# and don't have a C# compiler right now so the syntax might be invalid.)

Answer 2

1. Contact and ContactItem should in some way be child and parent, or at-least share a common ancestor so that you don't have to manually copy over all that identical contact information.

2. I'm not certain, but I think you might have wanted all this to be done with the LogicalName and not the LastName of this contact.

3. You should be using string.IsNullOrWhiteSpace in-case these strings haven't been trimmed or something.

4. As for simplifying your nest of ifs there are two solutions that fit the program as is.

   1. Use a less ugly ternary statement to do exactly what you were doing before

      if(string.IsNullOrWhiteSpace(LastName))
      {
          LastName = 
              !String.IsNullOrEmpty(contactItem.FirstName) ? contactItem.FirstName : 
              !String.IsNullOrEmpty(contactItem.FullName) ? contactItem.FullName : 
              !String.IsNullOrEmpty(contactItem.CompanyName) ? contactItem.CompanyName : 
              !String.IsNullOrEmpty(contactItem.Email1Address) ? contactItem.Email1Address : 
              @"Naam niet bekend";
          if(LastName == FirstName)
              FirstName = string.Empty;
      }
      

   2. Using Linq: I created a list of all the possible values that you considered valid, then calling the First() Extension method on our string array, would by default return just the first item in our array. By passing in a lambda expression I override that behavior to return the first value which matches my condition, which is !string.IsNullOrWhiteSpace()

      if (string.IsNullOrWhiteSpace(LastName))
      {
          LastName = new string[] { contactItem.FirstName, contactItem.FullName, contactItem.CompanyName, contactItem.Email1Address, @"Naam niet bekend" }
              .First(str => !string.IsNullOrWhiteSpace(str));
          if (LastName == FirstName)
              FirstName = string.Empty;
      }
      

5. If you were certain that all your empty strings were actually null instead of empty strings you could have just used the null-coalescing operator ??, which returns the first side of the argument which proves to not be null while moving from left to right. In this case, your code would have looked like this.

   Note: To make a string null use string str = default(string);

   if (string.IsNullOrWhiteSpace(LastName))
   {
       LastName = contactItem.FirstName ?? contactItem.FullName ?? contactItem.CompanyName ?? contactItem.Email1Address ?? @"Naam niet bekend";
       if (LastName == FirstName)
           FirstName = string.Empty;
   }
   

6. The code I posted in 4.1 is the most efficient because it only does as many comparisons as is needed, and cuts out when it finds the correct value. One advantage 4.1 has over the code you posted is that it doesn't do a string assignment before it does the comparison. Your code could have been re-written to avoid that as-well.

   The code in 4.2 is sleek, however it does require you to create an array, which is not expensive at all.

   I am only mentioning this, because it is something to consider when choosing the right code for you. While the differences here may not even be noticeable by the computer its self in terms of run-time speed, efficiency is something to keep in mind at all times when coding.

Answer 3

Take advantage of inline assignment and the short-circuiting behavior of the && operator. This code has the same behavior as your original, but it collapses the innermost three if-statements together into one:

if (String.IsNullOrEmpty(LastName))
{
    if (String.IsNullOrEmpty(LastName = contactItem.FirstName))
    {
        if (String.IsNullOrEmpty(LastName = contactItem.FullName)
            && String.IsNullOrEmpty(LastName = contactItem.CompanyName)
            && String.IsNullOrEmpty(LastName = contactItem.Email1Address))
        {
            LastName = @"Naam niet bekend";
        }
    }
    else
    {
        FirstName = "";
    }
}

Answer 4

You could reverse the condition, and build an if - else - if ladder. Only the worst case scenario would have 5 evaluations.

...
    if (String.IsNullOrEmpty(LastName))
    {
         if (!string.IsNullOrEmpty(contactItem.FirstName))
         {
                LastName = ...
         }
         else if (!string.IsNullOrEmpty(contactItem.FullName))
         {
                LastName = ...
         }
         else if (!string.IsNullOrEmpty(contactItem.CompanyName))
         {
                LastName = ...
         }
         else if (!string.IsNullOrEmpty(contactItem.Email1Address))
         {
                LastName = ...
         }
         else
         {
                LastName = ...
         }
    }
...