5
\$\begingroup\$

This code works, but I get the feeling that it could be shorter and simpler. Any ideas?

private void CheckNameCase(Models.PersonalInformationModels.PersonalInformationModel model)
{
   // if first middle and last names are all upper or lower case then title case the name

    //Name

    string firstName = model.NameModel.FirstName;
    string upperFirstName;
    string lowerFirstName;
    if (firstName == null)
    {
         upperFirstName = null;
         lowerFirstName = null;
    }
    else
    {
         upperFirstName = model.NameModel.FirstName.ToUpper(new CultureInfo("en-US", false));
         lowerFirstName = model.NameModel.FirstName.ToLower(new CultureInfo("en-US", false));
    }

    string middleName = model.NameModel.MiddleName;
    string upperMiddleName;
    string lowerMiddleName;
    if (middleName == null)
    {
         upperMiddleName = null;
         lowerMiddleName = null;
    }
    else 
    {
         upperMiddleName = model.NameModel.MiddleName.ToUpper(new CultureInfo("en-US", false));
         lowerMiddleName = model.NameModel.MiddleName.ToLower(new CultureInfo("en-US", false));            
    }
    string lastName = model.NameModel.LastName;
    string upperLastName;
    string lowerLastName;
    if (lastName == null)
    {
         upperLastName = null;
         lowerLastName = null;   
    } else
    { 
         upperLastName = model.NameModel.LastName.ToUpper(new CultureInfo("en-US", false));
         lowerLastName = model.NameModel.LastName.ToLower(new CultureInfo("en-US", false));         
    }


    if ((firstName == upperFirstName || firstName == lowerFirstName) && (middleName == upperMiddleName || middleName == lowerMiddleName) && (lastName == upperLastName || lastName == lowerLastName))
    {
        if(!String.IsNullOrEmpty(model.NameModel.FirstName)){ model.NameModel.FirstName = System.Threading.Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(firstName.ToLower(new CultureInfo("en-US", false))); }
        if(!String.IsNullOrEmpty(model.NameModel.MiddleName)){ model.NameModel.MiddleName = System.Threading.Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(middleName.ToLower(new CultureInfo("en-US", false))); }
        if(!String.IsNullOrEmpty(model.NameModel.LastName)){model.NameModel.LastName = System.Threading.Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(lastName.ToLower(new CultureInfo("en-US", false))); }
    }


    //Alternate Name

    string altFirstName = model.AlternateNameModel.FirstName;
    string upperAltFirstName;
    string lowerAltFirstName;
    if(altFirstName==null)
    {
         upperAltFirstName = null;
         lowerAltFirstName = null;            
    }else
    {
         upperAltFirstName = model.AlternateNameModel.FirstName.ToUpper(new CultureInfo("en-US", false));
         lowerAltFirstName = model.AlternateNameModel.FirstName.ToLower(new CultureInfo("en-US", false));
    }


    string altMiddleName = model.AlternateNameModel.MiddleName;
    string upperAltMiddleName;
    string lowerAltMiddleName;
    if(altMiddleName==null)
    {
         upperAltMiddleName = null;
         lowerAltMiddleName = null;            
    }else
    {
         upperAltMiddleName = model.AlternateNameModel.MiddleName.ToUpper(new CultureInfo("en-US", false));
         lowerAltMiddleName = model.AlternateNameModel.MiddleName.ToLower(new CultureInfo("en-US", false));           
    }


    string altLastName = model.AlternateNameModel.LastName;
    string upperAltLastName;
    string lowerAltLastName;
    if (altLastName==null) 
    {
         upperAltLastName = null;
         lowerAltLastName = null;            
    } else 
    { 
         upperAltLastName = model.AlternateNameModel.LastName.ToUpper(new CultureInfo("en-US", false));
         lowerAltLastName = model.AlternateNameModel.LastName.ToLower(new CultureInfo("en-US", false));
    }

    if ((altFirstName == upperAltFirstName || altFirstName == lowerAltFirstName) && (altMiddleName == upperAltMiddleName || altMiddleName == lowerAltMiddleName) && (altLastName == upperAltLastName || altLastName == lowerAltLastName))
    {
         if(!String.IsNullOrEmpty(model.AlternateNameModel.FirstName)){model.AlternateNameModel.FirstName = System.Threading.Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(altFirstName.ToLower(new CultureInfo("en-US", false)));}
         if(!String.IsNullOrEmpty(model.AlternateNameModel.MiddleName)){model.AlternateNameModel.MiddleName = System.Threading.Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(altMiddleName.ToLower(new CultureInfo("en-US", false)));}
         if(!String.IsNullOrEmpty(model.AlternateNameModel.LastName)){ model.AlternateNameModel.LastName = System.Threading.Thread.CurrentThread.CurrentCulture.TextInfo.ToTitleCase(altLastName.ToLower(new CultureInfo("en-US", false))); }
    }

}
\$\endgroup\$
6
\$\begingroup\$

Good layout and spacing.

My first observation is that you are repeating yourself many times over. If you look closely, all of the checks are the same. If you find duplicate code, move them out into a function.

Second observation is you are coding lengthy namespaces. Learn about the using ; syntax at the top of your file. Will save tonnes of tracing through.

Also try to avoid nesting if statements. Nested if's really clutter up the code and make it hard to follow.

Here is they way I'd clean up up:

private class UpperAndLowerString
{
    public string UpperCase { get; set; }
    public string LowerCase { get; set; }

    public UpperAndLowerString(string name)
    {
        UpperCase = name == null ? null : name.ToUpper(new CultureInfo("en-US", false));
        LowerCase = name == null ? null : name.ToLower(new CultureInfo("en-US", false));
    }

    public bool EqualsUpperCase(string check)
    {
        return UpperCase == check;
    }

    public bool EqualsLowerCase(string check)
    {
        return LowerCase == check;
    }

    public bool Equals(string check)
    {
        return EqualsUpperCase(check) || EqualsLowerCase(check);
    }
}

private static string ConvertToTitleCase(string name)
{
    return string.IsNullOrEmpty(name) ? null : TextInfo.ToTitleCase(name);
}


private static string ProcessSingleName(string name)
{
    var casedName = new UpperAndLowerString(name);

    return casedName.Equals(name) ? ConvertToTitleCase(name) : name;
}

private void CheckNameCase(Models.PersonalInformationModels.PersonalInformationModel model)
{
    model.NameModel.FirstName = ProcessSingleName(model.NameModel.FirstName) ?? model.NameModel.FirstName;
    model.NameModel.MiddleName = ProcessSingleName(model.NameModel.MiddleName) ?? model.NameModel.MiddleName;
    model.NameModel.LastName = ProcessSingleName(model.NameModel.LastName) ?? model.NameModel.LastName ;

    model.AlternateNameModel.FirstName = ProcessSingleName(model.AlternateNameModel.FirstName) ?? model.AlternateNameModel.FirstName;
    model.AlternateNameModel.MiddleName = ProcessSingleName(model.AlternateNameModel.MiddleName) ?? model.AlternateNameModel.MiddleName;
    model.AlternateNameModel.LastName = ProcessSingleName(model.AlternateNameModel.LastName) ?? model.AlternateNameModel.LastName;

}
\$\endgroup\$
3
  • \$\begingroup\$ +1 Good comments. Made some further suggestions in an answer as too much for a comment. \$\endgroup\$
    – dreza
    Aug 28 '12 at 21:35
  • \$\begingroup\$ It looks like you are checking and title casing each name part individually. It only needs to be title cased if the first middle and last are all three upper or lower cased by the user. \$\endgroup\$ Aug 29 '12 at 13:32
  • \$\begingroup\$ Oops, missed that part. You can modify the logic, but the basic principles should be the same. \$\endgroup\$ Aug 29 '12 at 15:37
2
\$\begingroup\$

This is more a suggested slight tweak to the excellant ideas and solution provided by Jeff. I would potentially look adjusting the ProcessedSingleName method as below

private static string ProcessSingleName(string name)
{
   var processedName = string.Empty;

   if(casedName.Equals(name))
   {
      processedName = ConvertToTitleCase(name);
   }

   return processedName ?? name;
}

Then the CheckNameCase would be adjusted to:

private void CheckNameCase(Models.PersonalInformationModels.PersonalInformationModel model)
{
    model.NameModel.FirstName = ProcessSingleName(model.NameModel.FirstName);
    model.NameModel.MiddleName = ProcessSingleName(model.NameModel.MiddleName);
    model.NameModel.LastName = ProcessSingleName(model.NameModel.LastName);

    model.AlternateNameModel.FirstName = ProcessSingleName(model.AlternateNameModel.FirstName);
    model.AlternateNameModel.MiddleName = ProcessSingleName(model.AlternateNameModel.MiddleName);
    model.AlternateNameModel.LastName = ProcessSingleName(model.AlternateNameModel.LastName);
}

Further thoughts: During typing this I thought you could even look at making an extension method on the PersonalInformationModel class (or on the class itself maybe?) to do the name conversion:

public static void NameCase(this PersonalInformationModel person, Func<string, string> processName)
{
    person.NameModel.FirstName = processName(model.NameModel.FirstName);
    person.NameModel.MiddleName = processName(model.NameModel.MiddleName);
    person.NameModel.LastName = processName(model.NameModel.LastName);  
}

Which would further lead to refactoring of CheckNameCase():

private void CheckNameCase(Models.PersonalInformationModels.PersonalInformationModel model)
{
    model.NameModel.NameCase(ProcessSingleName);
    model.AlternateNameModel.NameCase(ProcessSingleName);   
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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