snippetcsharpModerate
How can I make this method shorter and easier to follow?
Viewed 0 times
thiscanmethodmakefollowhowandshortereasier
Problem
It's long, repetitive, and hard to follow.
```
private void Map(object x, JToken json)
{
var objType = x.GetType();
var props = objType.GetProperties().Where(p => p.CanWrite).ToList();
foreach (var prop in props)
{
var type = prop.PropertyType;
var name = prop.Name;
var value = json[name];
var actualName = name;
if (value == null)
{
// try camel cased name
actualName = name.ToCamelCase(Culture);
value = json[actualName];
}
if (value == null)
{
// try lower cased name
actualName = name.ToLower(Culture);
value = json[actualName];
}
if (value == null)
{
// try name with underscores
actualName = name.AddUnderscores();
value = json[actualName];
}
if (value == null)
{
// try name with underscores with lower case
actualName = name.AddUnderscores().ToLower(Culture);
value = json[actualName];
}
if (value == null)
{
// try name with dashes
actualName = name.AddDashes();
value = json[actualName];
}
if (value == null)
{
// try name with dashes with lower case
actualName = name.AddDashes().ToLower(Culture);
value = json[actualName];
}
if (value == null || value.Type == JTokenType.Null)
{
continue;
}
// check for nullable and extract underlying type
if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))
{
type = type.GetGenericArguments()[0];
}
```
private void Map(object x, JToken json)
{
var objType = x.GetType();
var props = objType.GetProperties().Where(p => p.CanWrite).ToList();
foreach (var prop in props)
{
var type = prop.PropertyType;
var name = prop.Name;
var value = json[name];
var actualName = name;
if (value == null)
{
// try camel cased name
actualName = name.ToCamelCase(Culture);
value = json[actualName];
}
if (value == null)
{
// try lower cased name
actualName = name.ToLower(Culture);
value = json[actualName];
}
if (value == null)
{
// try name with underscores
actualName = name.AddUnderscores();
value = json[actualName];
}
if (value == null)
{
// try name with underscores with lower case
actualName = name.AddUnderscores().ToLower(Culture);
value = json[actualName];
}
if (value == null)
{
// try name with dashes
actualName = name.AddDashes();
value = json[actualName];
}
if (value == null)
{
// try name with dashes with lower case
actualName = name.AddDashes().ToLower(Culture);
value = json[actualName];
}
if (value == null || value.Type == JTokenType.Null)
{
continue;
}
// check for nullable and extract underlying type
if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))
{
type = type.GetGenericArguments()[0];
}
Solution
To expand on Carl's answer a little more, this would be a good candidate for the strategy pattern. You could add each case as a strategy callback:
So you're passing a list of matching strategies in order of execution, with the function returning the result of the first successful matching strategy.
Or you could encapsulate each strategy into its own class - which would enable you to unit test them individually.
private var toValue(var name, params Func[] matchingStrategies)
{
foreach(var strategy in matchingStrategies)
{
value = json[strategy(name)];
if (value != null)
return value;
}
return null;
}
// usage:
var name = prop.Name;
var value = toValue(name,
(n) => n.ToCamelCase(Culture),
(n) => n.ToLower(Culture),
(n) => n.AddUnderscores(),
(n) => n.AddUnderscores().ToLower(Culture),
(n) => n.AddDashes(),
(n) => n.AddDashes().ToLower(),
(n) => n.ToCamelCase(Culture));So you're passing a list of matching strategies in order of execution, with the function returning the result of the first successful matching strategy.
Or you could encapsulate each strategy into its own class - which would enable you to unit test them individually.
Code Snippets
private var toValue(var name, params Func<var, var>[] matchingStrategies)
{
foreach(var strategy in matchingStrategies)
{
value = json[strategy(name)];
if (value != null)
return value;
}
return null;
}
// usage:
var name = prop.Name;
var value = toValue(name,
(n) => n.ToCamelCase(Culture),
(n) => n.ToLower(Culture),
(n) => n.AddUnderscores(),
(n) => n.AddUnderscores().ToLower(Culture),
(n) => n.AddDashes(),
(n) => n.AddDashes().ToLower(),
(n) => n.ToCamelCase(Culture));Context
StackExchange Code Review Q#5450, answer score: 19
Revisions (0)
No revisions yet.