Is the new version of this code more difficult to read and understand than the original? How can it be improved? I was asked to do a code review for the following block of code. It implemented a bug fix to prevent some values being added to some drop down lists depending on the user's domain, in an ASP.NET WebForms app. The three lines adding items to the lists were from the pre-bug-fix code (not part of the fix), so I didn't review it.

        //Only add corporate AD domain if the user is a member of it.
        string domainName = "";
        if (Thread.CurrentPrincipal == null) 
        {
            //"Missing the thread identity";
        }
        else if (Thread.CurrentPrincipal.Identity.Name.IndexOf("\\") &lt; 0)
        {
            //"No domain detected leave domain name blank.";
        }
        else
        {
            domainName = Utils.GetUserDomain(Thread.CurrentPrincipal).ToUpper();
        }

        if (domainName == Config.AppSettings.Domain.ToUpper())
        {
              domainUserDropDownList.Items.Add(new ListItem(Config.AppSettings.Domain, Config.AppSettings.Domain));
              domainOfficeDropDownList.Items.Add(new ListItem(Config.AppSettings.Domain, Config.AppSettings.Domain));
              domainTypeDropDownList.Items.Add(new ListItem(Config.AppSettings.Domain, Config.AppSettings.Domain));
        }


I reviewed the code and suggested changing the code to the following:

        //Only add corporate AD domain if the user is a member of it.
        var currentPrincipal = Thread.CurrentPrincipal;
        var hasDomain = currentPrincipal != null &amp;&amp; currentPrincipal.Identity.Name.Contains("\\");
        var domainName = hasDomain ? Utils.GetUserDomain(currentPrincipal) : null;

        if (string.Equals(domainName, Config.AppSettings.Domain, StringComparison.OrdinalIgnoreCase))
        {
              domainUserDropDownList.Items.Add(new ListItem(Config.AppSettings.Domain, Config.AppSettings.Domain));
              domainOfficeDropDownList.Items.Add(new ListItem(Config.AppSettings.Domain, Config.AppSettings.Domain));
              domainTypeDropDownList.Items.Add(new ListItem(Config.AppSettings.Domain, Config.AppSettings.Domain));
        }


When the original developer saw the changes, he agreed with some of it (change of IndexOf to Contains, and ToUpper to String.Equals for case insensitive search), but not all of it. His main complaint was that the new code was more difficult to read and understand, and that the if blocks were clearer than the ternary operator. We weren't able to come to a point of agreement.

Could you please review both the original and new code and suggest improvements to either. Also, please point out any problems in the new code, especially where you think it is worse than the original code.
Disclaimer: I'm not too familiar with C#.

The modified form has expressions that are a little too complex for me, but either would be ok. My only comment is that the initial comment line is screaming for some notice:

// Only add corporate AD domain if the user is a member of it.


Why not take all the tests and put them into a function?

if ( IsMemberOfCorporateDomain( Thread.CurrentPrincipal ) ) ...


Then, depending on how you both feel about early exits:

private bool IsMemberOfCorporateDomain( IPrincipal currentPrincipal )
{
    if ( currentPrincipal == null ) return false;
    if ( !currentPrincipal.Identity.Name.Contains("\\") ) return false;

    var domainName = Utils.GetUserDomain(currentPrincipal);
    return string.Equals( domainName, Config.AppsSettings.Domain, StringComparison.OrdinalIgnoreCase )
}