HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
serializinghelperstringpropertynetforwhenjsonclasschange

Problem

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;
    }
}

Solution

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 {
                             { "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();
 }

Code Snippets

//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;
}
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;
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 }
                         };
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;
    }
}
[JsonProperty(PropertyName = "9")]
public string Title
{
    get
    {
        return _title;
    }
    set
    {
        _title = value;
        _title = GetTitleId(Title).ToString();
    }
}

Context

StackExchange Code Review Q#44369, answer score: 4

Revisions (0)

No revisions yet.