patterncsharpMinor
Name proper casing
Viewed 0 times
propercasingname
Problem
I deal with hundreds and hundreds of names each week. These names (along with other pedigree information) are stored in a database. Typically, I get these names in all sorts of formats, mainly proper and in all-caps. I needed an easy way to convert the names (especially last names) to their proper format:
The following code properly formats the above examples as well as standard names. Please take a look at my code and suggest areas where I might make it more efficient. I got the idea for this method mainly from this link.
```
public string ConvertToProperNameCase(string input)
{
bool SuffixProcessed = false;
input = input.Trim();
if (String.IsNullOrEmpty(input))
{
return String.Empty;
}
char[] chars = CultureInfo.CurrentCulture.TextInfo.ToTitleCase(input.ToLower()).ToCharArray();
for (int i = 0; i + 1 4))
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 2).ToLower() + s.Substring(3, 1).ToUpper() + s.Substring(4).ToLower();
}
catch
{
s = s;
}
}
if ((s.ToUpper().StartsWith("MC")) && s.Length > 3)
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 1).ToLower() + s.Substring(2, 1).ToUpper() + s.Substring(3).ToLower();
}
catch
{
s = s;
}
}
if (s.ToUpper().Contains(" III") && !SuffixProcessed)
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" III")) + " " + s.Substring(s.ToUpper().IndexOf(" III"), 4).ToUpper();
SuffixProcessed = true;
}
catch
{
s = s;
}
}
if (s.ToUpper().Contains(" II") && !SuffixProcessed)
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" II")) + " " + s.Substring(s.ToUpper().IndexOf(" II"), 3).ToUpper();
SuffixProcessed = true;
- McDonald
- MacDougal
- Smith-Jones
- Davis II
- George IV
The following code properly formats the above examples as well as standard names. Please take a look at my code and suggest areas where I might make it more efficient. I got the idea for this method mainly from this link.
```
public string ConvertToProperNameCase(string input)
{
bool SuffixProcessed = false;
input = input.Trim();
if (String.IsNullOrEmpty(input))
{
return String.Empty;
}
char[] chars = CultureInfo.CurrentCulture.TextInfo.ToTitleCase(input.ToLower()).ToCharArray();
for (int i = 0; i + 1 4))
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 2).ToLower() + s.Substring(3, 1).ToUpper() + s.Substring(4).ToLower();
}
catch
{
s = s;
}
}
if ((s.ToUpper().StartsWith("MC")) && s.Length > 3)
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 1).ToLower() + s.Substring(2, 1).ToUpper() + s.Substring(3).ToLower();
}
catch
{
s = s;
}
}
if (s.ToUpper().Contains(" III") && !SuffixProcessed)
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" III")) + " " + s.Substring(s.ToUpper().IndexOf(" III"), 4).ToUpper();
SuffixProcessed = true;
}
catch
{
s = s;
}
}
if (s.ToUpper().Contains(" II") && !SuffixProcessed)
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" II")) + " " + s.Substring(s.ToUpper().IndexOf(" II"), 3).ToUpper();
SuffixProcessed = true;
Solution
public string ConvertToProperNameCase(string input)
{
bool SuffixProcessed = false;You don't use this variable for a long time, move it closer to where it is used.
input = input.Trim();
if (String.IsNullOrEmpty(input))
{
return String.Empty;
}
char[] chars = CultureInfo.CurrentCulture.TextInfo.ToTitleCase(input.ToLower()).ToCharArray();Rather then Title casing now, why don't you make everything lowercase here, selectively uppercase pieces and then have the whole thing TitleCased? That you can assume everything is lowercase throughout this function which will simplify the code.
for (int i = 0; i + 1 < chars.Length; i++)
{
if ((chars[i].Equals('\'')) ||
(chars[i].Equals('-')))
{
chars[i + 1] = Char.ToUpper(chars[i + 1]);
}
}
string s = new string(chars);Why don't you create an array of bools, the same length as your string. Initially have all values set to false. Set those values to true which should be uppercased. Apply the uppercasing as the last part of the function.
if (((s.ToUpper().StartsWith("MAC") || s.ToUpper().StartsWith("MCC") || s.ToUpper().StartsWith("DE ")) && s.Length > 4))
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 2).ToLower() + s.Substring(3, 1).ToUpper() + s.Substring(4).ToLower();It would be a lot simpler just to set a flag in the should_uppercase array to true.
}
catchDon't just catch any error. Catch the specific type of error that will be thrown. Otherwise you might catch something you didn't mean to.
{
s = s;This statement does nothing.
}
}
if ((s.ToUpper().StartsWith("MC")) && s.Length > 3)
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 1).ToLower() + s.Substring(2, 1).ToUpper() + s.Substring(3).ToLower();
}
catch
{
s = s;
}
}You've done essentially the same thing twice. You should put all of these "mac, mc, de, mcc" in a constant array and then loop over it do to that logic.
if (s.ToUpper().Contains(" III") && !SuffixProcessed)What if " III" appears somewhere that isn't the end? Do you still want to do this?
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" III")) + " " + s.Substring(s.ToUpper().IndexOf(" III"), 4).ToUpper();
SuffixProcessed = true;
}
catchCan this actually happen? If you don't think its going to happen don't catch it as an exception and ignore it. That'll just mask bugs.
{
s = s;
}
}
if (s.ToUpper().Contains(" II") && !SuffixProcessed)
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" II")) + " " + s.Substring(s.ToUpper().IndexOf(" II"), 3).ToUpper();
SuffixProcessed = true;
}
catch
{
s = s;
}
}
if (s.ToUpper().Contains(" IV") && !SuffixProcessed)
{
try
{
s = s.Substring(0, s.ToUpper().IndexOf(" IV")) + " " + s.Substring(s.ToUpper().IndexOf(" IV"), 3).ToUpper();
SuffixProcessed = true;
}
catch
{
s = s;
}
}You should at least put these roman numerals in an array so you don't have to duplicate your logic. But what if Louis XIV decides to to use your system? You might want to consider something like checking whether the last word in the name consists only of X,I,V, or something.
return s;
}Round 2
public string ConvertToProperNameCase(string s)
{
string ReturnValue = string.Empty;You don't use ReturnValue until way later (aside from a couple places where you shouldn't be using it) so move it where you actually need it.
if (s.Trim().Length > 0)You end up repeatedly trimming s. Can you just trim it once and be done with it? Do you need to preserve whitespace on either end?
{
try
{
s = s.ToLower();
bool[] Name = new bool[s.Length];
for (int i = Name.Length; i < Name.Length; i++)
{
Name[i] = false;
}Name may not be the best choice, because the variable doesn't indicate that it is keeping track of cases.
for (int i = 0; i < s.Length; i++)
{
if (i == 0)
{
Name[i] = true;
}Just set
Name[0] = true; before entering this loop. ```
if (s[i].Equals('\'') || s[i].Equals('-') || s[i].Equals(' '))
{
Name[i + 1] = true;
}
}
string[] Prefixes = new string[] { "MAC", "MC" };
bool[] b = new bool[1] { true };
foreach (string p in Prefixes)
Code Snippets
public string ConvertToProperNameCase(string input)
{
bool SuffixProcessed = false;input = input.Trim();
if (String.IsNullOrEmpty(input))
{
return String.Empty;
}
char[] chars = CultureInfo.CurrentCulture.TextInfo.ToTitleCase(input.ToLower()).ToCharArray();for (int i = 0; i + 1 < chars.Length; i++)
{
if ((chars[i].Equals('\'')) ||
(chars[i].Equals('-')))
{
chars[i + 1] = Char.ToUpper(chars[i + 1]);
}
}
string s = new string(chars);if (((s.ToUpper().StartsWith("MAC") || s.ToUpper().StartsWith("MCC") || s.ToUpper().StartsWith("DE ")) && s.Length > 4))
{
try
{
s = s.Substring(0, 1).ToUpper() + s.Substring(1, 2).ToLower() + s.Substring(3, 1).ToUpper() + s.Substring(4).ToLower();}
catchContext
StackExchange Code Review Q#5248, answer score: 7
Revisions (0)
No revisions yet.