Click here to Skip to main content
14,269,998 members
Rate this:
Please Sign up or sign in to vote.
See more:
I use if else statements in my programs to make sure all the values are enteredthe user for. If the inner-most 'if statement' fails the subsequent else statements are executed and I'm not sure if its the right way of programming.Here is my code:
void MethodA()
{
if (validate(s_startDate) == true && validate(s_endDate)== true) 
  {
    if(s_jobNum.IsEmpty() != true && s_jobLoc.IsEmpty() != true) 
    {
      if(height.Length() != 0  )
      {
        //Do some calculations  
             
      }
      else  Evo::Alarm(23, ALM_NOTIFY, _T("Please enter the height!"));
    }
    else  Evo::Alarm(23, ALM_NOTIFY, _T("Please enter job number and location!"));
  }
  else  Evo::Alarm(23, ALM_NOTIFY, _T("Please enter the start and end dates in the correct format!"));
 
}


I have used the if loops to check if the user has enetered the correct values abd if they are in the right format.
I checked this in debug and found that:-
If the starting 2 loops are true inner most : loop - " height.Length() != 0 " is checked and is not valid, then the loop goes into the remaining 2 else statements as well.
Is this the proper way or the format should be like this
if{}
else
if{}
else
if {}
else


* validate is a method returns true or false after checking the date formats
Posted
Comments
chandanadhikari 10-Oct-12 10:38am
   
hi,
i think readability is your concern here. The second format will be more appropriate in cases where you have, say several if-else conditions. Then keeping them nested might not be a good idea so, a better approach will be to follow the second format where in every else block you can put a return statement after printing the message.i feel your code is quite readable to a level of three nested if-else.Had there been more then second format would have been a better choice.
Also i would prefer to use some local variables for the comparisons instead of directly using the function for evaluating the condition . (Please pardon me if i got your question wrong)
Sumal.V 10-Oct-12 10:48am
   
No no..U got my question right.It is solved now.
Rate this:
Please Sign up or sign in to vote.

Solution 1

I can't see anything that would cause the problem you describe, but I would do it a different way anyway:
if (!(validate(s_startDate) == true && validate(s_endDate)== true))
   {
   Evo::Alarm(23, ALM_NOTIFY, _T("Please enter the start and end dates in the correct format!"));
   return;
   }
if (!((s_jobNum.IsEmpty() != true && s_jobLoc.IsEmpty() != true))
   {
   Evo::Alarm(23, ALM_NOTIFY, _T("Please enter job number and location!"));
   return;
   }
if (height.Length() == 0)
   {
   Evo::Alarm(23, ALM_NOTIFY, _T("Please enter the height!"));
   return;
   }
// Do your calculations
Some people will tell you that there should only be a single exit, but I prefer to do all my validity checking in one place, and exit immediately.
   
Comments
CPallini 10-Oct-12 10:29am
   
"but I prefer to do all my validity checking in one place, and exit immediately."
I completely agree on this.
Sergey Alexandrovich Kryukov 10-Oct-12 16:28pm
   
Agree, this is convenient and reliable style.
--SA
CPallini 10-Oct-12 16:48pm
   
Uh?
The three of us agree? Beeeeer!
:-)
Sergey Alexandrovich Kryukov 10-Oct-12 16:57pm
   
Cheers!
--SA
Sumal.V 10-Oct-12 10:43am
   
Okay, just thought it was a bit dodgy.. But I changed it to this format just now.
Sumal.V 10-Oct-12 10:43am
   
Thanks
OriginalGriff 10-Oct-12 11:55am
   
You're welcome!
chandanadhikari 10-Oct-12 10:48am
   
5! do as the Griff says
Sergey Alexandrovich Kryukov 10-Oct-12 16:28pm
   
5ed.
--SA
Rate this:
Please Sign up or sign in to vote.

Solution 2

If the most inner if clause fails (it is evaluated false) then only the first else clause will be executed (the one that follows the if clause condition) the other else clauses will not be executed.

Probably you believe that because when compiling you are seeing the program pointer going on the line you have the else clause (which in your programming style is the same where you have the contents of the else clause).

But trust me, if those else clauses are executed after going into the most inner if clause... then some kind of magic happens.

Your syntax is correct (in terms of if / else) I've not checked anything else.

Good luck!
   
Comments
Sumal.V 10-Oct-12 10:45am
   
Exactly! that's the main reason I thought I was wrong! As i was checking in the debug mode and found the pointer going to the rest of the else statements..
But hey thanks.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100