after validating looped if statement else statement is not working

Question

I have a if elseif else statement like as follows

if (check1)   ---- 1
{
    if(check2)   ----2
    {
    }
}
else if (checkother1) ---- 3
{
    if(checkother2) -----4
}
else  ------- 5
{
    do this
}
return someview

when the code hits first if statement and if the condition validates it will go inside check2 if statement after doing the validation it is entering the last else statement(5) correctly .

But when the code enters the 3rd if statement and after validating the 4th if statement if is not validated it is showing error properly but if it get validated it is directly going to the return

[HttpPost]
        [ValidateAntiForgeryToken]
        public async Task<IActionResult> Edit(int id, [Bind("CanditateId,CanditateFirstName,CanditateLastName,CanditateAadhar,CanditatePancard,CanditateEmail,CanditateGender,CanditateLocation,CanditateMaritialStatus,CanditateDoj,CanditateActDoa,CanditateDesignation,CanditateDepartment,CanditateTeam,CanditateAddress,CanditateAltNumber,CanditateIdNumber,CanditateBaseLocation,CanditateManager,CanditatePrefix,CanditateMobile")] Canditateinfo canditateinfo)
        {
            var canditateinfos = await _context.Canditateinfos.FindAsync(id);
            if (id != canditateinfo.CanditateId)
            {
                return NotFound();
            }

            
            if (ModelState.IsValid)
            {
                try 
                {
                    var isAAdharAlreadyExists = _context.Canditateinfos.Any(x => x.CanditateAadhar == canditateinfo.CanditateAadhar);
                    var isPanCardAlreadyExists = _context.Canditateinfos.Any(x => x.CanditatePancard == canditateinfo.CanditatePancard);
                    if (canditateinfos.CanditateAadhar != canditateinfo.CanditateAadhar)
                    {
                        if (isAAdharAlreadyExists)
                        {
                            ModelState.AddModelError("CanditateAadhar", "User with this aadhar already exists");
                            return View(canditateinfo);
                        }

                    }
                    else if (canditateinfos.CanditatePancard != canditateinfo.CanditatePancard)
                    {
                        if (isPanCardAlreadyExists)
                        {
                            ModelState.AddModelError("CanditatePancard", "User with this Pan Number already exists");
                            return View(canditateinfo);
                        }

                    }
                    else
                    {
                        canditateinfo.EnteredBy = canditateinfos.EnteredBy;
                        canditateinfo.EnteredDate = canditateinfos.EnteredDate;
                        canditateinfo.DeleteFlag = canditateinfos.DeleteFlag;
                        canditateinfo.IsActive = canditateinfos.IsActive;
                        canditateinfo.CanditateId = canditateinfos.CanditateId;
                        canditateinfo.UpdatedBy = HttpContext.Session.GetString("username");
                        canditateinfo.UpdatedDate = DateTime.Now;
                        _context.Update(canditateinfo);
                        await _context.SaveChangesAsync();
                        TempData["SuccessMessage"] = "Canditate Updated Successfully";
                        return RedirectToAction(nameof(Index));
                    }
                }
                catch 
                {
                }
                
            }
            return View(canditateinfo);
        }

https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems — Selvin, Dec 01 '22 at 12:57
also obviously ... your else would not be called if 1st true and 2nd false or 3th true and 4th false ... so it basically means that if you change pancard or aadhar and they are not already in database would be never updated but also there will be no error — Selvin, Dec 01 '22 at 13:01
5 is never executed if either 1 or 3 succeeds. The inner checks 2 and 4 do not matter for 5 to be executed or not. — Olivier Jacot-Descombes, Dec 01 '22 at 13:07
@OlivierJacot-Descombes The code deviates from the simplyfied pseudo code in that it has `return` statements in the nested `if`s. I layed that out in my answer for OP. — Fildor, Dec 01 '22 at 13:47
Unrelated: What if `!ModelState.IsValid`? You might want to spare yourself one level of nesting and handle this with a `if(!ModelState.IsValid) return ...;` and then go on to code the rest of the method assuming the ModelState _is_ valid. — Fildor, Dec 01 '22 at 13:54

Fildor · Accepted Answer · 2022-12-01T14:07:20.387

Nested ifs do not matter for the outer control flow. (Yours do, though but hear me out ;D )

In ...

if ( condition1 )
{
   // body1
}
else if( condition2 )
{
   // body2
}
else
{
   // body3
}
// aftermath

... "body3" will be executed if and only if neither condition1 nor condition2 evaluate to true.

That is regardless of any further nested if statements in "body1" or "body2". (Given, there are no fancy gotos or returns or the like... - more on this further down)

That also means:

If condition1 is true, the flow is "body1", "aftermath" (Note that in this case, it doesn't even matter what condition2 evals to.)
If condition1 is false and condition2 is true, the flow is "body2", "aftermath"
If condition1 is false and condition2 is false, the flow is "body3", "aftermath"

Now, in your code, you do have returns inside the inner if statements. This of course does influence the control flow.

They will "bail out" early. That means ...

if (condition1)
{
    if (condition1_1)
    {
        return retVal1; // If 1 && 1_1 => Rest of method is not executed
    }
//  if !condition1_1 we end up at "aftermath" for the the next instruction 
//  because condition1 was true!!
} 
else if (condition2)
{
    if (condition2_1)
    {
        return retVal2; // If 2 && 2_2 => Rest of the method is not executed
    }
//  if !condition2_1 we end up at "aftermath" for the the next instruction 
//  because condition2 was true!!
} 
else
{
    // body3
}

// aftermath

So, if you want "body3" to be executed in those cases where the nested ifs do not bail out, you need to restructure this logic.

As far as I can see, those are sanity tests. I think if you do something like this, it should work as expected:

if (condition1 && condition1_1)
{ // here, BOTH primary AND secondary condition MUST evaluate to true
  // so that this branch is entered
    return retVal1; // If 1 && 1_1 => Rest of method is not executed
} 
else if (condition2 && condition2_1)
{
    return retVal2; // If 2 && 2_2 => Rest of the method is not executed
} 
else
{
    // body3
}

which can be simplyfied to

if (condition1 && condition1_1)
{
    return retVal1; // If 1 && 1_1 => Rest of method is not executed
} 

if (condition2 && condition2_1)
{
    return retVal2; // If 2 && 2_2 => Rest of the method is not executed
} 

// what was before body3

Note that in if (condition1 && condition1_1) C#/.NET is so kind to evaluate condition1_1 only if condition1 evaluates to true. Otherwise the whole expression evaluates to false. Which is correct: If condition1 is false already, then condition1 && condition1_1 can never be true regardless of what condition1_1 would evaluate to, so it can stop early.

So, it is safe to write something like if( x is not null && x.IsValid ).

Fun fact:

Your original code would probably work as expected without the elses:

if( condition1 )
{
  if ( subcondition1 )
  {
// condition && subcondition == true
     return errorCase1;
  }
}

// you can land here if one of condition1 or subcondition1 is false
// and therefore go on to execute :
if( condition2 )
{
  if ( subcondition2 )
  {
     return errorCase2;
  }
}

// happy path here, aka "body3"

after validating looped if statement else statement is not working

1 Answers1