Recent

Author Topic: [Solved] Nested If Then Else statement  (Read 10539 times)

Rayvenhaus

  • Jr. Member
  • **
  • Posts: 70
[Solved] Nested If Then Else statement
« on: January 19, 2017, 03:32:19 am »
I'm completely stuck with this nested if then else statement. I keep getting a syntax error that says "wyckermain.pas(727,5) Fatal: Syntax error, ";" expected but "ELSE" found at the first Else statement. I've searched, I've twisted, I've even sacrificed a small dust bunny, all to no avail. Can anyone point out the simple mistake I've made, that I can't see?


Code: Pascal  [Select][+][-]
  1. procedure Tfrm_wyckermain.MenuItem14Click(Sender: TObject);
  2. begin
  3.   //Check for Wycker Update
  4.   if WyckerLogLevel = 1 then
  5.     Logger.Info(sLoggerChkUpd);
  6.   If LazAutoUpdate1.NewVersionAvailable Then
  7.     If LazAutoUpdate1.DownloadNewVersion Then
  8.       Begin
  9.         If MessageDlg(Application.Title, sUpdDownloadComplete, mtInformation, [mbOK], 0) = 1 then
  10.           LazAutoUpdate1.UpdateToNewVersion
  11.       End;
  12.     Else
  13.       if WyckerLogLevel = 1 then
  14.         Logger.Info(sLoggerChkUpdFailed);
  15.       ShowMessage(sUpdDownloadFailed)
  16.   Else
  17.     if WyckerLogLevel = 1 then
  18.       Logger.Info(sLoggerChkUpdNone);
  19.     ShowMessage(sUpdNoUpdate);
  20. end;                  
  21.  
« Last Edit: January 19, 2017, 01:41:00 pm by Rayvenhaus »

Handoko

  • Hero Member
  • *****
  • Posts: 5129
  • My goal: build my own game engine using Lazarus
Re: Nest If Then Else statement
« Reply #1 on: January 19, 2017, 03:50:44 am »
I tested your code, it can't compile. Except after I edited line no. 11:

"End;"   --->  "End"

edit:
Oops, I forgot to mention there also need a begin-end block for line no. 13-15. Thanks Molly and J-G for the correction.
« Last Edit: January 19, 2017, 07:56:45 am by Handoko »

molly

  • Hero Member
  • *****
  • Posts: 2330
Re: Nest If Then Else statement
« Reply #2 on: January 19, 2017, 04:00:53 am »
I tested your code, it can't compile. Except after I edited line no. 11:

"End;"   ---> "End"
And the million dollar question: is that the way the code is/was intended to run ?

Mostly the else parts, it is unclear to which if statement they are intended to belong to. Only TS is able to tell that, the code for sure doesn't  (or actually it does, but again was it intended) :)

btw: only changing  what you wrote @handoku then i get an error in line 16: "Fatal: Syntax error, ";" expected but "ELSE" found"

@\Rayvenhaus:
make things more clear for both the compiler and yourself by putting your nested if statements between begin.. end blocks.
« Last Edit: January 19, 2017, 04:16:01 am by molly »

J-G

  • Hero Member
  • *****
  • Posts: 953
Re: Nest If Then Else statement
« Reply #3 on: January 19, 2017, 04:14:13 am »
I'm completely stuck with this nested if then else statement. I keep getting a syntax error that says "wyckermain.pas(727,5) Fatal: Syntax error, ";" expected but "ELSE" found at the first Else statement. I've searched, I've twisted, I've even sacrificed a small dust bunny, all to no avail. Can anyone point out the simple mistake I've made, that I can't see?
Code: Pascal  [Select][+][-]
  1. procedure Tfrm_wyckermain.MenuItem14Click(Sender: TObject);
  2. begin
  3.   //Check for Wycker Update
  4.   if WyckerLogLevel = 1 then
  5.      Logger.Info(sLoggerChkUpd);
  6.         If LazAutoUpdate1.NewVersionAvailable Then
  7.            If LazAutoUpdate1.DownloadNewVersion Then
  8.              Begin
  9.                 If MessageDlg(Application.Title, sUpdDownloadComplete, mtInformation, [mbOK], 0) = 1 then
  10.                   LazAutoUpdate1.UpdateToNewVersion
  11.             End     ;
  12.          Else
  13.            if WyckerLogLevel = 1 then
  14.               Logger.Info(sLoggerChkUpdFailed);
  15.            ShowMessage(sUpdDownloadFailed)
  16.         Else
  17.           if WyckerLogLevel = 1 then
  18.             Logger.Info(sLoggerChkUpdNone);
  19.        ShowMessage(sUpdNoUpdate);
  20. end;                  
  21.  

The error message tells you EXACTLY what your simple mistake is - I've highlighted it in red. Unfortunately that doesn't work in 'code' segments, so I'll have to specify  -  line 11 ends in a semicolon (I've added space so you can see it) but that MUST NOT appear before the ELSE directive (in line 12).  At line 15 you haven't terminated with a semi-colon (correctly).

I find it helps to indent code and I've adusted your code in the way I think you mean but as Molly has said (while I was typing) your code is not very clear.

FPC 3.0.0 - Lazarus 1.6 &
FPC 3.2.2  - Lazarus 2.2.0 
Win 7 Ult 64

Handoko

  • Hero Member
  • *****
  • Posts: 5129
  • My goal: build my own game engine using Lazarus
Re: Nest If Then Else statement
« Reply #4 on: January 19, 2017, 04:17:28 am »
For complex nested conditions, I usually use Exit to simplify the logic.

I would recommend something like this:

Code: Pascal  [Select][+][-]
  1. Procedure CheckUpdate;
  2. begin
  3.  
  4.   If not(NewVersionAvaiable) then begin
  5.     ShowMessage('No Update');
  6.     Exit;
  7.   end;
  8.  
  9.   If (UserCancelUpdate) then begin
  10.     ShowMessage('Update is postponed.');
  11.     Exit;
  12.   end;
  13.  
  14.   if (UpdateSuccess) then
  15.     ShowMessage('App has updated.')
  16.   else
  17.     ShowMessage('Update failed.');
  18.  
  19. end;
  20.  
« Last Edit: January 19, 2017, 04:20:51 am by Handoko »

molly

  • Hero Member
  • *****
  • Posts: 2330
Re: Nest If Then Else statement
« Reply #5 on: January 19, 2017, 04:28:55 am »
Reading the code a bit more carefully (as in actually understanding what it is suppose to do, but taking my guesses there) i think TS meant (intended) for this to happen:
Code: Pascal  [Select][+][-]
  1. procedure LogInfo; begin end;
  2. procedure Updater; begin end;
  3. procedure ShowMsg; begin end;
  4. function  WantUpdate: boolean; begin result := true end;
  5. function  LoglevelOk: boolean; begin result := true end;
  6. function  NewVersion: boolean; begin result := true end;
  7. function  Downloaded: boolean; begin result := true end;
  8.  
  9. procedure B;
  10. begin
  11.   //Check for Wycker Update
  12.   if LoglevelOK then LogInfo;       // sLoggerChkUpd
  13.  
  14.   // new version available ?
  15.   if NewVersion Then
  16.   begin
  17.     // Did download new version succeed ?
  18.     if Downloaded Then
  19.     Begin
  20.       // If user wants to update then update
  21.       if WantUpdate then Updater
  22.     End
  23.     Else // Download of new version failed
  24.     begin
  25.       if LoglevelOK then LogInfo;   // sLoggerChkUpdFailed
  26.       ShowMsg;                      // sUpdDownloadFailed
  27.     end;
  28.   end
  29.   else // No new version available
  30.   begin
  31.     if LoglevelOK then LogInfo;     // sLoggerChkUpdNone
  32.     ShowMsg;                        // sUpdNoUpdate
  33.   end;
  34. end;
  35.  
edit: added more meaningful names for routines and added comments to make things more clear.

The only thing out of the ordinary with my approach (but perhaps TS's problem as well) is that there is logged that checkupdate failed, and directly thereafter is a message to user shown that download failed. It should be the one or the other.
« Last Edit: January 19, 2017, 05:03:22 am by molly »

minesadorada

  • Sr. Member
  • ****
  • Posts: 452
  • Retired
Re: Nest If Then Else statement
« Reply #6 on: January 19, 2017, 08:45:22 am »
@Rayvenhaus just an observation:  If you do
Code: Pascal  [Select][+][-]
  1. LazAutoUpdate1.DebugMode:=TRUE;
then you can use the built-in OnDebugEvent method to log activity:
Code: Pascal  [Select][+][-]
  1. procedure Tmainform.LazAutoUpdate1DebugEvent(Sender: TObject; lauMethodName,
  2.   lauMessage: string);
  3. begin
  4.    Logger.Info('('+lauMethodName+') - ' + lauMessage);
  5. end;

There are also events fired: OnDownloaded and OnNewVersionAvailable and the property LastError with info that you might use.  And if you use the single method AutoUpdate, it offers the user choices and dialogs (which are i8n-friendly).  AutoUpdate internally is a similar big if-then-else statement to the one you're building.

==Added this info to the Wiki page http://wiki.freepascal.org/LazAutoUpdater==
« Last Edit: January 19, 2017, 09:37:04 am by minesadorada »
GPL Apps: Health MonitorRetro Ski Run
OnlinePackageManager Components: LazAutoUpdate, LongTimer, PoweredBy, ScrollText, PlaySound, CryptINI

Rayvenhaus

  • Jr. Member
  • **
  • Posts: 70
Re: [Solved] Nested If Then Else statement
« Reply #7 on: January 19, 2017, 01:49:28 pm »
The simplest fix was this:

Code: Pascal  [Select][+][-]
  1. procedure Tfrm_wyckermain.MenuItem14Click(Sender: TObject);
  2. begin
  3.   //Check for Wycker Update
  4.   if WyckerLogLevel = 1 then
  5.     Logger.Info(sLoggerChkUpd);
  6.   If LazAutoUpdate1.NewVersionAvailable Then
  7.     If LazAutoUpdate1.DownloadNewVersion Then
  8.       Begin
  9.         If MessageDlg(Application.Title, sUpdDownloadComplete, mtInformation, [mbOK], 0) = 1 then
  10.           LazAutoUpdate1.UpdateToNewVersion
  11.       End  //Removed ";"
  12.     Else
  13.       begin  // Added Begin/End
  14.         if WyckerLogLevel = 1 then
  15.           Logger.Info(sLoggerChkUpdFailed);
  16.         ShowMessage(sUpdDownloadFailed)
  17.       end
  18.   Else
  19.     if WyckerLogLevel = 1 then
  20.       Logger.Info(sLoggerChkUpdNone);
  21.     ShowMessage(sUpdNoUpdate);
  22. end;                  
  23.  

Thanks to all that helped, this was driving me crazy!!!!

lainz

  • Hero Member
  • *****
  • Posts: 4460
    • https://lainz.github.io/
Re: Nest If Then Else statement
« Reply #8 on: January 19, 2017, 02:34:58 pm »
@\Rayvenhaus:
make things more clear for both the compiler and yourself by putting your nested if statements between begin.. end blocks.

I agree, I'ts solved but I can't easily read it.

Handoko

  • Hero Member
  • *****
  • Posts: 5129
  • My goal: build my own game engine using Lazarus
Re: [Solved] Nested If Then Else statement
« Reply #9 on: January 19, 2017, 03:02:37 pm »
I found the most visible problem that causes it hard to read is the code wasn't indented properly. The fixed version has fixed the indentation issue. But I usually avoid no more than 2 levels of if-then-else condition.

creaothceann

  • Full Member
  • ***
  • Posts: 117
Re: [Solved] Nested If Then Else statement
« Reply #10 on: January 19, 2017, 10:12:03 pm »
The simplest fix was this:

Code: Pascal  [Select][+][-]
  1. procedure Tfrm_wyckermain.MenuItem14Click(Sender: TObject);
  2. begin
  3.   //Check for Wycker Update
  4.   if WyckerLogLevel = 1 then
  5.     Logger.Info(sLoggerChkUpd);
  6.   If LazAutoUpdate1.NewVersionAvailable Then
  7.     If LazAutoUpdate1.DownloadNewVersion Then
  8.       Begin
  9.         If MessageDlg(Application.Title, sUpdDownloadComplete, mtInformation, [mbOK], 0) = 1 then
  10.           LazAutoUpdate1.UpdateToNewVersion
  11.       End  //Removed ";"
  12.     Else
  13.       begin  // Added Begin/End
  14.         if WyckerLogLevel = 1 then
  15.           Logger.Info(sLoggerChkUpdFailed);
  16.         ShowMessage(sUpdDownloadFailed)
  17.       end
  18.   Else
  19.     if WyckerLogLevel = 1 then
  20.       Logger.Info(sLoggerChkUpdNone);
  21.     ShowMessage(sUpdNoUpdate);
  22. end;                  
  23.  

Thanks to all that helped, this was driving me crazy!!!!
That code is wrong; it always executes the "ShowMessage(sUpdNoUpdate)" line.

Fixed version:

Code: [Select]
procedure TFrm_Wyckermain.MenuItem14Click(Sender : TObject);
begin
if (WyckerLogLevel = 1) then Logger.Info(sLoggerChkUpd);
if LazAutoUpdate1.NewVersionAvailable then begin
        if LazAutoUpdate1.DownloadNewVersion then begin
                if (MessageDlg(Application.Title, sUpdDownloadComplete, mtInformation, [mbOK], 0) = 1) then LazAutoUpdate1.UpdateToNewVersion;
        end else begin
                if (WyckerLogLevel = 1) then Logger.Info(sLoggerChkUpdFailed);
                ShowMessage(sUpdDownloadFailed);
        end;
end else begin
        if (WyckerLogLevel = 1) then Logger.Info(sLoggerChkUpdNone);
        ShowMessage(sUpdNoUpdate);
end;
end;

Or, the easier to understand (imo) version:

Code: [Select]
procedure TFrm_Wyckermain.MenuItem14Click(Sender : TObject);
begin
if (WyckerLogLevel = 1) then Logger.Info(sLoggerChkUpd);
if (not LazAutoUpdate1.NewVersionAvailable) then begin
        if (WyckerLogLevel = 1) then Logger.Info(sLoggerChkUpdNone);
        ShowMessage(sUpdNoUpdate);
        exit;
end;
if (not LazAutoUpdate1.DownloadNewVersion) then begin
        if (WyckerLogLevel = 1) then Logger.Info(sLoggerChkUpdFailed);
        ShowMessage(sUpdDownloadFailed);
        exit;
end;
if (MessageDlg(Application.Title, sUpdDownloadComplete, mtInformation, [mbOK], 0) <> 1) then exit;
LazAutoUpdate1.UpdateToNewVersion;
end;

Handoko

  • Hero Member
  • *****
  • Posts: 5129
  • My goal: build my own game engine using Lazarus
Re: [Solved] Nested If Then Else statement
« Reply #11 on: January 20, 2017, 04:28:17 am »
+1 creaothceann

That code is wrong; it always executes the "ShowMessage(sUpdNoUpdate)" line.

 

TinyPortal © 2005-2018