C# Nested If Statements in TDD and Code Metrics [closed]

Question

public void AccessPermissions(User user)
{
   if (user.Age >= 18)
   {
      if (user.IsRegistred)
      {
         if (user.IsPowerfull)
         {
             AcessGrantLevel3();
         }
         else
         {
             AccessGrantLevel2();
         }
      }
      else
      {
         AccessGrantLevel1();
      }
   }
   else
   {
       AcceessDenied();
   }
}

According to code metrics measurement this function is very bad and according to the clean code developemnt this function is bad for testing. What can I do to make Code Metrics and TDD happy? Is nested If statements always bad?

Answer 1

public void AccessPermissions(User user)
{
    if(user.Age<18)
    {
        AccessDenied();
        return;
    }
    if(user.IsPowerfull && user.IsRegistered)
    {
        AccessGrantLevel3();
        return;
    }
    if(user.IsRegistered)
    {
        AccessGrantLevel2();
        return;
    }
    AccessGrantLevel1();
    return;
}

Answer 2

You can flatten the logic like this:

public void AccessPermissions(User user)
{
    if (user.Age < 18)
    {
        AcceessDenied();
    }
    else if (!user.IsRegistred)
    {
        AccessGrantLevel1();
    }
    else if (!user.IsPowerfull)
    {
        AcessGrantLevel2();
    }
    else
    {
        AccessGrantLevel3();
    }
}

Answer 3

It can easily be re-written without nesting.

public void AccessPermissions(User user)
{
   if (user.Age < 18)
   {
       AcceessDenied();
   }
   else if (!user.IsRegistred)
   {
       AccessGrantLevel1();
   }
   else if (user.IsPowerfull)
   {
       AcessGrantLevel3();
   }
   else
   {
       AccessGrantLevel2();
   }
}

Answer 4

This is a matter of preference, but most people consider nested if statements to be less readable and harder to maintain, because the code that is to be executed is far away from the statement that caused it.

By reversing the condition of the If statement, you can move the execution logic closer to the condition. consider:

public void AccessPermissions(User user)
{
    if (!(user.Age >= 18))
    {
        AcccessDenied();
    }
    else if (!user.IsRegistered)
    {
        AccessGrantLevel1();
    }
    else if (!user.IsPowerfull)
    {
        AccessGrantLevel2();
    }
    else
    {
        AccessGrantLevel3();
    }
}

Ultimately, any of the multiple examples on this page of other approaches would work with a bit of tweaking... the concept still stands that your goal should be to reduce or eliminate nesting by inverting your conditions.

Answer 5

Here is one approach

public void AccessPermissions(User user)
{
    int UserLevel = 0;

UserLevel += (user.Age>=18) ? 1:0;      // Add 1 if user is over 18
UserLevel += (user.IsIsRegistred) ? 1:0;    // If registered, add another 1
UserLevel += (user.IsPowerFull) ? 1:0;      // Add another 1 if powerful

AccessGrantLevel( UserLevel );
}