Search code examples
c#design-patternspolymorphismconditional-statementsstrategy-pattern

Replacing conditional with something?


I have the following nested if clause, I was wondering if I, by applying some pattern, can simplify it?

The code checks to see if it needs an authorizationStartDate, and if it does, but does not have it, returns true.

I've considered the Strategy Pattern, the "Replace conditional with polymorphism" method, the Specification Pattern and various others, but I haven't found anything which I liked.

private bool IsMissingAuthorizationStartDate(ApplicationStatusData data)
    {
        if (data.ApplicationStatus == ApplicationStatusType.ApplicationApproved)
        {
            if (data.ApplicationPurpose == ApplicationPurpose.New)
            {
                if (data.ProductStatus?.ProductStatusType == ProductStatusType.ApplicationForNewProductReceived)
                {
                    if (data.ApplicationTypePesticide == ApplicationTypePesticide.Authorisation ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.ProvisionalAuthorisation ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.MutualRecognition ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.Derogation ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.DispensationPreviousAssessment ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.ResearchAndDevelopmentPurposesExperimentOrTestProgram ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.ResearchAndDevelopmentPurposesExperimentOrTestProgramKnownProduct ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.ParallelTradePermit ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.Copy
                        )
                    {
                        if (!data.AuthorizationStartDate.HasValue)
                        {
                            return true;
                        }
                    }
                }
            }
            else if (data.ApplicationPurpose == ApplicationPurpose.Renewal)
            {
                if (data.ProductStatus.ProductStatusType == ProductStatusType.ProductAuthorised)
                {
                    if (data.ApplicationTypePesticide == ApplicationTypePesticide.ReAuthorisation ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.ParallelTradePermit ||
                        data.ApplicationTypePesticide == ApplicationTypePesticide.Copy
                        )
                    {
                        if (!data.AuthorizationStartDate.HasValue)
                        {
                            return true;
                        }
                    }
                }
            }
        }

        // else
        return false;
    }

Solution

  • The pattern I would use here is just encapsulation. The nesting here is hard to follow, and worsened by the equality comparisons. If possible, instead of exposing the raw field, try encapsulating the intent.

    e.g. Instead of if (data.ApplicationPurpose == ApplicationPurpose.Renewal) try extending ApplicationStatusData with a property like

    bool IsRenewalApplication 
    {
     get 
     {
      return this.ApplicationPurpose == ApplicationPurpose.Renewal;
     }
    }
    

    so your code reads cleaner, with more expression: if (data.IsRenewalApplication) { ... }

    Particularly where you have that massive if this or that or that or that, put it under a well-named property like IsInterestingPesticide.

    If you can't change ApplicationStatusData for some reason, you can do the same thing with member functions that return Boolean values, expressing the same intent.

    HTH!

    PS, You might even want to encapsulate the entirety of the nested-ifs into a single concept. Then you'd just have 2 Boolean tests before you return false.