0

First, I was haveing combobox1 fill combobox2 during the onselect. I started the long way see below.

    procedure TFGetZoneDept.ComboBox1Select(Sender: TObject);
begin
  Combobox2.Clear;
  with Combobox1 do
  begin
      if text = '3' then
      begin
        with combobox2 do
        begin
          Add('Zone 3 depts');
          Add('Zone 3 depts');
          Add('Zone 3 depts');
          Add('Zone 3 depts');
          Add('Zone 3 depts');
          Add('Zone 3 depts');
        end;   {with combobox2}
      end;  {If }
      if text = '4' then
      begin
        with ComboBox2 do
        begin
          add('Zone 4 depts');
          add('Zone 4 depts');
          add('Zone 4 depts');
          add('Zone 4 depts');
          add('Zone 4 depts)';
        end;{combobox2 with}
      end;{IF}
      if text ='1' then
      begin
        with ComboBox2 do
        begin
          add('Zone 1 depts');
          add('Zone 1 depts');
          add('Zone 1 depts');
          add('Zone 1 depts');
          add('Zone 1 depts');
          add('Zone 1 depts');
        end; {combobox2 with}
      end; {IF}
      if text ='2' then
      begin
        with ComboBox2 do
        begin
          add('Zone 2 depts');
          add('Zone 2 depts');
          add('Zone 2 depts');
          add('Zone 2 depts');
          add('Zone 2 depts');
          add('Zone 2 depts');
        end; {Combobox2 with}
      end; {IF}
      if text ='BoneYard' then
      begin
        with ComboBox2 do
        begin
          add('BoneYard depts');
          add('BoneYard depts');
          add('BoneYard depts');
          add('BoneYard depts');
          add('BoneYard depts');
          add('BoneYard depts');
        end; {combobox2 with}
      end; {IF}
      if text = 'Misc' then
      begin
        with ComboBox2 do
          begin
            add('Misc Depts');
            add('Misc Depts');
            add('Misc Depts');
            add('Misc Depts');
            add('Misc Depts');
            add('Misc Depts');
          end; {combobox2 with}
      end; {IF}
  end;{combobox1 with}
  Combobox2.Enabled := true;
end;

I noticed you cant use one with with another with inside.. or am i doing it wrong. Second I started to think there has to be a better way :D So either answer is ok. Either how to fix the with or do this a better way.

Rob Kennedy
  • 161,384
  • 21
  • 275
  • 467
Glen Morse
  • 2,437
  • 8
  • 51
  • 102

2 Answers2

8

It's entirely possible to have nested with statements. It's generally not a good idea, since the badness of with statements compounds, but the compiler has no trouble interpreting the code. When resolving identifiers, the compiler simply works its way from the inner statement to the outer one until it finds an object that has the method or property it's looking for.

What the compiler finds may differ from what you expect it to find.


You can make your code considerably more concise by using some variables and loops to avoid repeating code, as well as to remove the need for a with statement.

procedure TFGetZoneDept.ComboBox1Select(Sender: TObject);
var
  text1, text2: string;
  i: Integer;
begin
  Combobox2.Clear;
  text1 := Combobox1.Text;
  if text1 = '3' then
    text2 := 'Zone 3 depots'
  else if text1 = '4' then
    text2 := 'Zone 4 depts'
  else if text ='1' then
    text2 := 'Zone 1 depts'
  else if text ='2' then
    text2 := 'Zone 2 depts'
  else if text ='BoneYard' then
    text2 := 'BoneYard depts'
  else if text = 'Misc' then
    text2 := 'Misc Depts';
  for i := 1 to 6 do
    Combobox2.Items.add(text2);
  Combobox2.Enabled := true;
end;

When you avoid repeating code, you also avoid mistakes. Unless you meant for all the cases to have six options except zone 4, which only has five.

Rob Kennedy
  • 161,384
  • 21
  • 275
  • 467
  • ahh, after your assurance that you can.. i went back and saw i was doing Combobox2 instead of Combobox2.items Thanks.. also you think for this case its ok? This is all this function is doing, filling combobox 2? – Glen Morse Feb 08 '13 at 04:47
  • the options could be 20 down to 5 – Glen Morse Feb 08 '13 at 05:09
  • Then introduce another variable for the upper bound of the loop, and set it at the same time as `text2`. – Rob Kennedy Feb 08 '13 at 05:17
3

If you are utilizing nested With statements, punch yourself in the face to save time.

People have strong arguments over GOTOs and WITH statements. WITH statements are worse as they can be nested, compounding their evilness on every use.

The first rule of With, nobody talks about it (or uses it.)

Darian Miller
  • 7,808
  • 3
  • 43
  • 62
  • 2
    Nobody talks about it? We talk about it all the time. – Rob Kennedy Feb 08 '13 at 05:02
  • Lame "Fight Club" reference....First rule of Fight Club, is you don't talk about Fight Club. – Darian Miller Feb 08 '13 at 05:03
  • It's too dangerous and problematic to use. – Darian Miller Feb 08 '13 at 05:17
  • 2
    Glen, believe me, any time you save using with statements when writing code will be used up 10-fold when you come to debugging and maintaining it. – Keith Miller Feb 08 '13 at 08:21
  • 4
    @GlenMorse You don't spend your time typing. You spend your time debugging. What's more, if you would avoid repeating yourself like you do in the code in the Q, then that's how to reduce both typing and debugging. – David Heffernan Feb 08 '13 at 08:57
  • 1
    What costs more - creating code or maintaining code? If you have to post code like this in public because you cannot figure out what it's doing...all to save some keystrokes then your priorities are backwards. (And you should have classes to handle the population rather than so many references to controls..which would be the much better way of saving keystrokes!) – Darian Miller Feb 08 '13 at 20:36
  • with good comments, its pretty easy to keep track of the with's after every end; i put what its ending.. – Glen Morse Feb 12 '13 at 05:53
  • @Glen Then why are you posting a question on Stack Overflow if it is so easy? – Darian Miller Feb 12 '13 at 14:37