I'm having big trouble with reducing cognitive complexity in given piece of code. Could you please give some tips on how to fix this issue? I could reduce it from 24 to 16 using switch, but it is still 16 and I have no options left
protected override bool Compare(object valueToValidate, object valueToCompare)
{
if (RaUtils.IsBlankValue(valueToValidate) || RaUtils.IsBlankValue(valueToCompare))
{
return true;
}
switch (Type.GetTypeCode(valueToCompare.GetType()))
{
case TypeCode.DateTime:
if (DateTime.TryParse(valueToValidate.ToString(), out var valueToValidateDt)
&& DateTime.TryParse(valueToCompare.ToString(), out var valueToCompareDt))
{
return valueToValidateDt >= valueToCompareDt;
}
break;
case TypeCode.Double:
if (double.TryParse(valueToValidate.ToString(), out var valueToValidateDouble)
&& double.TryParse(valueToCompare.ToString(), out var valueToCompareDouble))
{
return valueToValidateDouble >= valueToCompareDouble;
}
break;
case TypeCode.Decimal:
if (decimal.TryParse(valueToValidate.ToString(), out var valueToValidateDecimal)
&& decimal.TryParse(valueToCompare.ToString(), out var valueToCompareDecimal))
{
return valueToValidateDecimal >= valueToCompareDecimal;
}
break;
case TypeCode.Int32:
if (int.TryParse(valueToValidate.ToString(), out var valueToValidateInt32)
&& int.TryParse(valueToCompare.ToString(), out var valueToCompareInt32))
{
return valueToValidateInt32 >= valueToCompareInt32;
}
break;
case TypeCode.Int64:
if (long.TryParse(valueToValidate.ToString(), out var valueToValidateInt64)
&& long.TryParse(valueToCompare.ToString(), out var valueToCompareInt64))
{
return valueToValidateInt64 >= valueToCompareInt64;
}
break;
default: throw new NotImplementedException();
}
return false;
}
You could try to move the code inside the case
blocks into their own methods:
private bool CompareDateTime(string value1, string value2)
{
if (DateTime.TryParse(value1, out var valueToValidateDt)
&& DateTime.TryParse(value2, out var valueToCompareDt))
{
return valueToValidateDt >= valueToCompareDt;
}
return false;
}
Your Comapre
would simplify to this:
protected override bool Compare(object valueToValidate, object valueToCompare)
{
if (RaUtils.IsBlankValue(valueToValidate) || RaUtils.IsBlankValue(valueToCompare))
{
return true;
}
switch (Type.GetTypeCode(valueToCompare.GetType()))
{
case TypeCode.DateTime:
return CompareDateTime(valueToValidate.ToString(), valueToCompare.ToString());
case TypeCode.Double:
return CompareDouble(valueToValidate.ToString(), valueToCompare.ToString());
case TypeCode.Decimal:
return CompareDecimal(valueToValidate.ToString(), valueToCompare.ToString());
case TypeCode.Int32:
return CompareInt32(valueToValidate.ToString(), valueToCompare.ToString());
case TypeCode.Int64:
return CompareInt64(valueToValidate.ToString(), valueToCompare.ToString());
default: throw new NotImplementedException();
}
}
Because all your methods have the same signature, you could also use a Dictionary<TypeCode,Func<string,string,bool>>
instead of a switch block:
protected override bool Compare(object valueToValidate, object valueToCompare)
{
if (RaUtils.IsBlankValue(valueToValidate) || RaUtils.IsBlankValue(valueToCompare))
{
return true;
}
var methods = new Dictionary<TypeCode,Func<string,string,bool>>
{
{ TypeCode.DateTime, CompareDateTime },
{ TypeCode.Double, CompareDouble },
{ TypeCode.Decimal, CompareDecimal },
{ TypeCode.Int32, CompareInt32 },
{ TypeCode.Int64, CompareInt64 }
};
if(methods.TryGetValue(Type.GetTypeCode(valueToCompare.GetType()), out var method))
{
return method.Invoke(valueToValidate.ToString(), valueToCompare.ToString());
}
else
{
throw new NotImplementedException();
}
}