I'm getting the following error from SonarCube: "Refactor this method to reduce its Cognitive Complexity from 28 to the 15 allowed". Thing is, with this high Cognitive Complexity score, I am not sure how to drive it down to required 15. I have tried to move three if statements to different methods, but it has lowered the score by only 6 pts. How should I refactor given code in order to meet the requirement?

 [HttpGet]
        public IActionResult GetTileViewer(string productId = "", string edgeId = "", string sizeId = "", double? thickness = null)
        {
            if (string.IsNullOrWhiteSpace(productId))
            {
                return new ObjectResult(new { Error = "You need to specify a product id" })
                    { StatusCode = (int)HttpStatusCode.InternalServerError };
            }

            var commerceService = new CommerceService();
            var isImperial = commerceService.IsImperialMeasure();

            var id = 0;
            Guid guid;
            RockstarProductTile product = null;

            if (Guid.TryParse(productId, out guid))
            {
                product = ProductUtilities.GetProduct<RockstarProductTile>(guid);
            }
            else if (int.TryParse(productId, out id))
            {
                product = ProductUtilities.GetProduct<RockstarProductTile>(id);
            }

            if (product != null)
            {
                double? length = null;
                double? width = null;

                try
                {
                    if (!string.IsNullOrWhiteSpace(sizeId))
                    {
                        var split = sizeId.Split('-');
                        length = double.Parse(split[0]);
                        width = double.Parse(split[1]);
                    }

                    var variants = product.Variants
                        .Where(x => string.IsNullOrWhiteSpace(edgeId) || commerceService.GetRelatedEntries(x, CommerceAssociationGroups.Edge)
                            .Where(y => y.ContentLink.ID.ToString() == edgeId || y.ContentGuid.ToString() == edgeId).Any());

                    if (isImperial)
                    {
                        variants = variants
                            .Where(x => length == null || x.Rockstar_RockstarProductTileVariant_TileLengthInches == length || x.Rockstar_RockstarProductTileVariant_TileLengthFeet == length)
                            .Where(x => width == null || x.Rockstar_RockstarProductTileVariant_TileWidthInches == width || x.Rockstar_RockstarProductTileVariant_TileWidthFeet == width)
                            .Where(x => thickness == null || x.Rockstar_RockstarProductTileVariant_TileThicknessInches == thickness || x.Rockstar_RockstarProductTileVariant_TileThicknessFeet == thickness);
                    }
                    else
                    {
                        variants = variants
                            .Where(x => length == null || x.Rockstar_RockstarProductTileVariant_TileLengthInches == length)
                            .Where(x => width == null || x.Rockstar_RockstarProductTileVariant_TileWidthInches == width)
                            .Where(x => thickness == null || x.Rockstar_RockstarProductTileVariant_TileThicknessInches == thickness);
                    }

                    var variant = variants.FirstOrDefault();

                    if (variant != null)
                    {
                        var tileViewer = commerceService.GetRelatedEntries(variant, CommerceAssociationGroups.TileViewer).FirstOrDefault() as RockstarTileViewer;
                        var images = tileViewer?.Rockstar_TileViewer_Images?.Items.Select(x => _urlUtilities.GetRelativeUrl(x.ContentLink)).ToList();

                        var tempId = 0;
                        Guid tempGuid;
                        RockstarProductEdge edge = null;

                        if (Guid.TryParse(edgeId, out tempGuid))
                        {
                            edge = ProductUtilities.GetProduct<RockstarProductEdge>(tempGuid);
                        }
                        else if (int.TryParse(edgeId, out tempId))
                        {
                            edge = ProductUtilities.GetProduct<RockstarProductEdge>(tempId);
                        }

                        if (images != null)
                        {
                            return new JsonResult(new TileViewerDataObject { Label = edge?.Rockstar_RockstarProductEdge_EdgeName?.SupAndSubOnly() ?? product.DisplayName, Images = images });
                        }

                        return new JsonResult(new TileViewerDataObject { Label = edge?.Rockstar_RockstarProductEdge_EdgeName?.SupAndSubOnly() ?? product.DisplayName });
                    }
                }
                catch (Exception ex)
                {
                    _logger.LogError(ex, "Error during get tile viewer: {Message}", ex.Message);
                }
            }

            return new JsonResult(new TileViewerDataObject());
        }

CodePudding user response：

AFAIK among the largest contributors to the cognitive complexity are nested expressions, so try reduce nesting. For example invert if - from if (product != null) to:

if (product == null)
{
    return new JsonResult(new TileViewerDataObject());
}

double? length = null; // also can be moved into the try block, not needed here
double? width = null;// also can be moved into the try block, not needed here

try
{
   ...
}
...

Then getting product should be moved to separate method (also for quite some time you can inline the variable declaration for out variables like id and guid) :

private RockstarProductTile GetProduct(string productId)
{
    RockstarProductTile result = null;
    if (Guid.TryParse(productId, out var guid))
    {
        product = ProductUtilities.GetProduct<RockstarProductTile>(guid);
    }
    else if (int.TryParse(productId, out var id))
    {
        product = ProductUtilities.GetProduct<RockstarProductTile>(id);
    }
    return result;
}

And in the method body just call it:

RockstarProductTile product = GetProduct(productId);

Then apply the same approaches to the try body (also possibly move to separate method) - try inverting at least some of the if's (if (variant != null) is a good candidate) and move getting edge by edgeId into separate method.

Also try to get methods to be less then one screen (by breaking down it in smaller ones, finding variant can be moved to separate function too).