Helper class to change property of string when serializing for Json.Net

Question

Is there any way to simplify the following C# helper class to numerate a string for Json.Net?

internal static string CreateContact(string title)
{
 var createContact = new {Title = title};
 string output = JsonConvert.SerializeObject(createContact);
 return output;
}
[JsonProperty(PropertyName = "9")]
public string Title
{
 get
 {
 return _title;
 }
 set
 {
 _title = value;
 _title = GetTitleId(Title).ToString();
 }
}
public static int? GetTitleId(string title)
{
 //remove "."
 var titleRemoveDots = title.Replace(".", "");
 var titleLowerCase = titleRemoveDots.Trim().ToLower();
 switch(titleLowerCase)
 {
 case "dir":
 return 1;
 case "dr": case "doctor":
 return 4;
 case "mag": case "magister":
 return 5;
 case "ing":
 return 6;
 case "dipling": case "dipl":
 return 7;
 case "prof": case "professor":
 return 8;
 case "dkfm":
 return 9;
 case "prok":
 return 12;
 default:
 return null;
 }
}

• Mathieu Guindon: How is the class named? Is there more to it?
• comecme: I don't get what you're trying to do in the Title setter. First you're assigning value to _title, then you are assigning the result of GetTitleId to _value. And you're passing the Title property in the method call. Also, I would normally expect the value of a property to have the value I've just set. Finally, the Title property doesn't contain a Title, but contains a TitleId.

Answer 1

Just a quick point, regarding readability; the switch block would be better off written like this:

//remove "."
var titleRemoveDots = title.Replace(".", "");
var titleLowerCase = titleRemoveDots.Trim().ToLower();
switch(titleLowerCase)
{
 case "dir":
 return 1;
 case "dr": 
 case "doctor":
 return 4;
 case "mag": 
 case "magister":
 return 5;
 case "ing":
 return 6;
 case "dipling": 
 case "dipl":
 return 7;
 case "prof": 
 case "professor":
 return 8;
 case "dkfm":
 return 9;
 case "prok":
 return 12;
 default:
 return null;
}

The absence of break; between cases (/ the presence of return in all cases) can make it harder to refactor the switch block later. I think it would be better to separate the concept of figuring out the return value and that of returning a value:

int? result;
//remove "."
var titleRemoveDots = title.Replace(".", "");
var titleLowerCase = titleRemoveDots.Trim().ToLower();
switch(titleLowerCase)
{
 case "dir":
 result = 1;
 break;
 case "dr": 
 case "doctor":
 result = 4;
 break;
 case "mag": 
 case "magister":
 result = 5;
 break;
 case "ing":
 result = 6;
 break;
 case "dipling": 
 case "dipl":
 result = 7;
 break;
 case "prof": 
 case "professor":
 result = 8;
 break;
 case "dkfm":
 result = 9;
 break;
 case "prok":
 result = 12;
 break;
 default:
 break;
}
return result;

Now, call me a switch-hater, I'd probably end up with something like this:

public enum PersonTitle
{
 Director = 1,
 Doctor = 4,
 Magister = 5,
 Ingineer = 6,
 Dipling = 7,
 Professor = 8,
 Dkfm = 9, // todo: give readable name
 Prok = 12 // todo: give readable name
}
private static _titleIds = new Dictionary<string, PersonTitle> {
 { "dir", PersonTitle.Director },
 { "dr", PersonTitle.Doctor },
 { "doctor", PersonTitle.Doctor },
 { "mag", PersonTitle.Magister },
 { "magister", PersonTitle.Magister },
 { "ing", PersonTitle.Ingineer },
 { "dipl", PersonTitle.Dipling },
 { "dipling", PersonTitle.Dipling },
 { "prof", PersonTitle.Professor },
 { "professor", PersonTitle.Professor },
 { "dkfm", PersonTitle.Dkfm },
 { "prok", PersonTitle.Prok }
 };

And then GetTitleId could look like this:

public static int? GetTitleId(string title)
{
 var cleanTitle = title.Replace(".", string.Empty).Trim().ToLower();
 PersonTitle result;
 if (_titleIds.TryGetValue(cleanTitle, out result))
 {
 return (int)result;
 }
 else
 {
 return null;
 }
}

One issue is that Title is actually a string representation of the int value, which can be allowed to be null - I might be mistaken, but it looks like the setter for Title would throw a NullReferenceException if/when that is the case, because it's calling ToString() on null:

[JsonProperty(PropertyName = "9")]
public string Title
{
 get
 {
 return _title;
 }
 set
 {
 _title = value;
 _title = GetTitleId(Title).ToString();
 }
}

I'd expect a null-check there:

 set
 {
 _title = value;
 _title = (GetTitleId(Title) ?? string.Empty).ToString();
 }

• user2151345: If anything this code is more complex than the code in my original question.
• Mathieu Guindon: @user2151345 indeed, a switch block with hard-coded magic numbers and strings is simpler to code. I find the dictionary simpler to maintain.