Recent

Author Topic: Code in If statement not executing; should be triggered by ComboBox select  (Read 643 times)

CarmichaelJohn

  • New Member
  • *
  • Posts: 40
Hello
I am trying to use a TComboBox to select the title of a value, then perform a simple multiplication using that value, however the code is not working. I am not sure what happened; this code worked in Delphi XE and I do not understand why it does not work in Lazarus:

It does not execute the code between the If... and end.

Code: Pascal  [Select][+][-]
  1. procedure TformSettlementValueEstimator.ComboBox1Select(Sender: TObject);
  2. var
  3.   x1, x2, value1, LHFactor1 : double;
  4.   Likelihood1 : String;
  5.  
  6. begin
  7.   //
  8.     value1 := StrToFloat(Edit1.Text);
  9.     Likelihood1 := ComboBox1.Text;
  10.     if Likelihood1 ='1 in 100' then
  11.                begin
  12.                    LHFactor1 := 0.01;
  13.                    x1 := Value1;
  14.                    x2 := x1 * LHFactor1;
  15.                    label9.caption := FloatToStrF(x2, ffNumber, 10, 0);
  16.                end
  17.        else if Likelihood1 = '2 in 100' then
  18.                begin
  19.                    LHFactor1 := 0.02;
  20.                    x1 := Value1;
  21.                    x2 := x1 * LHFactor1;
  22.                    label9.caption := FloatToStrF(x2, ffNumber, 10, 0);
  23.                end
  24.     else if Likelihood1 = '4 % or 1 in 25' then         {MORE CASES LIKE THE ABOVE}
  25.  

The Items list for the TComboBox is as follows:

Likelihood
1 in 100
1 in 50
1 in 25
1 in 20
1 in 10
1 in 5
1 in 4
1 in 2
3 in 5
3 in 4
4 in 5
9 in 10
win all


Thank you in advance for any help you can provide,
John
« Last Edit: January 21, 2021, 03:28:25 am by Martin_fr »

dbannon

  • Hero Member
  • *****
  • Posts: 1295
    • tomboy-ng, a rewrite of the classic Tomboy
John, should you use

Code: Pascal  [Select][+][-]
  1. Likelihood := combobox1.items[combobox1.ItemIndex]
?

Failing that, I think you need to check exactly what Likelihood contains just before that 'if' statement.  That will tell us if that section of code is being executed and may make the problem obvious.  Debugln or writeln are great if you are using Linux, otherwise, step through the code using the debugger.

Davo
« Last Edit: January 21, 2021, 03:25:08 am by dbannon »
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 6901
  • Debugger - SynEdit - and more
    • wiki
Edited original message: Added code tags.

@CarmichaelJohn
Please put code into [ code]  [ /code] tags

Handoko

  • Hero Member
  • *****
  • Posts: 4070
  • My goal: build my own game engine using Lazarus
I personally think using a case-select is better than multiple if-conditions.

Code: Pascal  [Select][+][-]
  1. begin
  2.   Value1 := StrToFloat(Edit1.Text);
  3.   case ComboBox1.Text of
  4.     '1 in 100':
  5.       begin
  6.         LHFactor1 := 0.01;
  7.         x1 := Value1;
  8.         x2 := x1 * LHFactor1;
  9.         Label9.Caption := FloatToStrF(x2, ffNumber, 10, 0);
  10.       end;
  11.     '2 in 100', '1 in 50':
  12.       begin
  13.         LHFactor1 := 0.02;
  14.         x1 := Value1;
  15.         x2 := x1 * LHFactor1;
  16.         Label9.Caption := FloatToStrF(x2, ffNumber, 10, 0);
  17.       end;
  18.     // more cases
  19.   end;
  20. end;

This even better, less code easier to maintain:

Code: Pascal  [Select][+][-]
  1. begin
  2.   Value1 := StrToFloat(Edit1.Text);
  3.   case ComboBox1.Text of
  4.     '1 in 100':            LHFactor1 := 0.01;
  5.     '2 in 100', '1 in 50': LHFactor1 := 0.02;
  6.     // more cases
  7.   end;
  8.   x1 := Value1;
  9.   x2 := x1 * LHFactor1;
  10.   Label9.Caption := FloatToStrF(x2, ffNumber, 10, 0);
  11. end;

I found something not correct in the original code. The compare string is '2 in 100' but in the combobox, it is '1 in 50'.
« Last Edit: January 21, 2021, 03:38:20 am by Handoko »

dbannon

  • Hero Member
  • *****
  • Posts: 1295
    • tomboy-ng, a rewrite of the classic Tomboy
Yes, nice code clean up Handoko !

But if the OP's code fails to execute even the first 'if', there is something else wrong.   (And no, its not as I suggested, combobox1.text does return the selected item, that surprises me, I expected it to return a string of all items, like TStrinList does).

But the case statement sure makes it easier to debug !

Davo
Lazarus 2, Linux (and reluctantly Win10, OSX)
My Project - https://github.com/tomboy-notes/tomboy-ng

Handoko

  • Hero Member
  • *****
  • Posts: 4070
  • My goal: build my own game engine using Lazarus
But if the OP's code fails to execute even the first 'if', there is something else wrong.

Yes, you're right.

The better solution should be:

Code: Pascal  [Select][+][-]
  1. begin
  2.   LHFactor1 := -1;
  3.   Value1    := StrToFloat(Edit1.Text);
  4.   case ComboBox1.Text of
  5.     '1 in 100':            LHFactor1 := 0.01;
  6.     '2 in 100', '1 in 50': LHFactor1 := 0.02;
  7.     // more cases
  8.   end;
  9.   if LHFactor1 > -1 then
  10.   begin
  11.     x1 := Value1;
  12.     x2 := x1 * LHFactor1;
  13.     Label9.Caption := FloatToStrF(x2, ffNumber, 10, 0);
  14.   end;
  15. end;

MarkMLl

  • Hero Member
  • *****
  • Posts: 2073
I agree about using case here, it eliminates a whole lot of possibilities for "dangling else" etc. particularly if the case wraps up using a catch-all "otherwise" rather than "else".

I checked this a few weeks ago, and case-on-string has been supported since FPC 2.6 so there's very little reason not to use it... particularly if the string being checked always has Trim() applied to it to help avoid mystifying bugs.

MarkMLl
Turbo Pascal v1 on CCP/M-86, multitasking with LAN and graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.

bytebites

  • Sr. Member
  • ****
  • Posts: 382
Case of string does not work in Delphi-mode.

MarkMLl

  • Hero Member
  • *****
  • Posts: 2073
Case of string does not work in Delphi-mode.

In that case the obscurity of this problem would merit moving the affected code to a unit which can be compiled in the default FPC mode where it /is/ supported.

An alternative, if you really /do/ have to use ifs, is something like

Code: Pascal  [Select][+][-]
  1. while true do begin
  2.   if something = somethingElse then begin
  3. ...
  4.     break
  5.   end;
  6. ...
  7.   begin // simulated "otherwise"
  8. ...
  9.     break
  10.   end;
  11.   Assert(false, 'Should never get here')
  12. end; // One-shot while block
  13.  

Now I'm obviously aware that that abuses Pascal syntax, but I suggest that in practical terms it's better than having an error-prone staircase of if-then-else statements.

"I'll get me coat" :-)

MarkMLl
« Last Edit: January 21, 2021, 01:26:33 pm by MarkMLl »
Turbo Pascal v1 on CCP/M-86, multitasking with LAN and graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.

Bart

  • Hero Member
  • *****
  • Posts: 4170
    • Bart en Mariska's Webstek
If the items in the ComboBox are hardcoded, then you can simply translate ItemIndex to LHFactor.
Have some const with meaningfull names defined like:
Code: Pascal  [Select][+][-]
  1. const
  2.   li1in100 = 0; //ItemIndex 0
  3.   li1in50 = 1;
  4.   //etc
  5. ...
  6.   Case ComboBox1Select.ItemIndex of
  7.     li1in100: LHFactor1 := 0.01;
  8.     li1in50: ... etc
  9.   end;//case

Bart

CarmichaelJohn

  • New Member
  • *
  • Posts: 40
Re: Code in If statement not executing; should be triggered by ComboBox select
« Reply #10 on: January 21, 2021, 08:50:15 pm »
Handoko and All who responded,
Thank you so very much.  The case option was cleaner than my code and it worked very well.  I always appreciate all of your comments and help each time I post a problem.  Thank you again.
John

 

TinyPortal © 2005-2018