Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

rule of thumb when coding large nested functional blocks

Tags:

c#

refactoring

I’ve been coding in c# for a while now and generally have a good idea about the rule of thumb for coding standards. I’ve been recently encourage by my colleges to adopt a results based approach to coding functional blocks rather than nesting blocks of logic and am seeking your advice. Below is an example of what I’m talking about, where the result of one situation is used to determine the code path rather than nesting. It’s been suggested that this approach is easier to read, especially if the result would require several layers of nesting, however I prefer to uses Curly’s Law and refactor methods and functions where nesting becomes to deep.

private void MethodOne()
    {

        bool CarryOn = false;

        // first layer
        if (ValidationRuleOne() == true)
        {
            CarryOn = true;

        } else {

            CarryOn = false;
        }

        // second layer
        if (CarryOn) 
        {
            CarryOn = ValidationRuleTwo();

        } else {

            CarryOn = false;
        }

        // third layer
        if (CarryOn)
        {
            CarryOn = ValidationRuleThree();

        } else
        {

            CarryOn = false;
        }

    }

This approach just seems wrong to me as I would suggest that the method should be rewriten as ..

        private void MethodOne()
    {



        // first layer
        if (ValidationRuleOne() == true)
        {

            // second layer
            if (ValidationRuleTwo() == true)
            {

                // third layer
                if (ValidationRuleThree() == true)
                {


                }

            }
        }

    }

If the nesting becomes too complex then I would suggest that the method/function structure needs to be rethought to group logical groups of functionality together into additional methods or functions are required?

Any thoughts greatly appreciated.

Regards,

Tim

like image 250
Tim Avatar asked Nov 28 '22 15:11

Tim


2 Answers

if (ValidationRuleOne() == true)
    {

        // second layer
        if (ValidationRuleTwo() == true)
        {

            // third layer
            if (ValidationRuleThree() == true)

can be:

if(ValidationRuleOne() && ValidationRuleTwo() && ValidationRuleThree())
{
...
}

in my opinion much easier to read.

and if for some reason you need each validation rule to fire:

if(ValidationRuleOne() & ValidationRuleTwo() & ValidationRuleThree())
{...} //note the & vs &&

(and you don't need to do if(validationRule() == true), since validation returns a boolean, you don't need to compare it to one in a conditional statement).

like image 109
kemiller2002 Avatar answered Dec 09 '22 17:12

kemiller2002


I personally like using guard clauses.

if (!ValidationRuleOne()) 
    return;

if (!ValidationRuleTwo())
    return;

if (!ValidationRuleThree()) 
    return;
like image 31
Jeremy Roberts Avatar answered Dec 09 '22 15:12

Jeremy Roberts