Search code examples
c#.net-coresonarqube

Reduce Cognitive Complexity


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

Solution

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