tags:

views:

1077

answers:

11

I been reading bad things about the with keyword in delphi but, in my opinion, if you don't over use it. It can make your code look simple.

I often put all my TClientDataSets and TFields in TDataModules. So in my forms I had code like this

procedure TMyForm.AddButtonClick(Sender: TObject);
begin  
  with LongNameDataModule do
  begin
     LongNameTable1.Insert;
     LongNameTable1_Field1.Value := "some value";
     LongNameTable1_Field2.Value := LongNameTable2_LongNameField1.Value;
     LongNameTable1_Field3.Value := LongNameTable3_LongNameField1.Value;
     LongNameTable1_Field4.Value := LongNameTable4_LongNameField1.Value;
     LongNameTable1.Post;
  end
end;

without the with keyword I have to write the code like this

    procedure TMyForm.AddButtonClick(Sender: TObject);
    begin            
      LongNameDataModule.LongNameTable1.Insert;
      LongNameDataModule.LongNameTable1_LongNameField1.Value := "some value";

      LongNameDataModule.LongNameTable1_LongNameField2.Value :=
               LongNameDataModule.LongNameTable2_LongNameField1.Value;

      LongNameDataModule.LongNameTable1_LongNameField3.Value :=
               LongNameDataModule.LongNameTable3_LongNameField1.Value;

      LongNameDataModule.LongNameTable1_LongNameField4.Value :=
               LongNameDataModule.LongNameTable4_LongNameField1.Value;

      LongNameDataModule.LongNameTable1.Post;
    end;

I think is easier to read using the with keyword.

Should I avoid using the with keyword?

+14  A: 

The "with" statement has it's place but I have to agree that overuse can lead to ambiguous code. A good rule of thumb is to make sure the code is "more" readable and maintainable after adding the with statement. if you feel you need to add comments to explain the code after adding the statement then it is probably a bad idea. If the code is more readable as in your example then use it.

btw: this was always one of my favorite patterns in Delphi for showing a modal window

with TForm.Create(nil) do
try
  ShowModal;
finally
  Free;
end
Jamey McElveen
Agreed. Using "with" is both a good and bad habit. After a while you'll learn where to draw the line.
Gerard
Another solution for modal forms would be adding a new static method "CreateShowModalAndFree" ;) ...
mjustin
+2  A: 

The main problem with "with" is that you don't know where its scope ends, and you could have multiple overlapping with statements.

I don't think you should avoid using it, as long as your code is readable.

One of the proposals to make it more readable (and less confusing in longer code) was if codegear added the option to allow for aliases in with, and probably allowing multiple withs in one:

procedure TMyForm.AddButtonClick(Sender: TObject);
begin  
  with LongNameDataModule as dm, dm.LongNameTable1 as t1, dm.LongNameTable2 as t2 do
  begin
    t1.Insert;
    t1.FieldByName('Field1').AsString := 'some value';
    t1.FieldByName('Field2').AsString := t2.FieldByName('Field2').AsString;
    t1.Post;
    dm.Connection.Commit;
  end
end;
Osama ALASSIRY
In the example I use TField instead of the FieldByName method, so I don't need an alias for LongNameTable2. I can just use dm.LongTable2_Field1.Value. Thanks for the feedback
Marioh
+2  A: 

As far as I'm concerned, With is quite acceptable in the case you give. It certainly improves the code clarity.

The real evil is when you have multiple with's open at once.

Also, my opinion is that what you are using the with on makes a big difference. If it's a truly different object then the with is probably a bad idea. However, I dislike having a lot of variables at one level even when this makes sense--generally data objects that hold an entire very complex data item--generally the entire piece of work the program is designed to work with. (I do not think this case would occur in an app that didn't have such an item.) To make the world clearer I often use records to group related items. I find that almost all withs I use are for accessing such subgroups.

Loren Pechtel
+6  A: 

When I first began pascal programming (with TurboPascal!) and learnt as I went, WITH seemed wonderful. As you say, the answer to tedious typing and ideal for those long records. Since Delphi arrived, I've been removing it and encouraging other to drop it - neatly summed-up by Verity at the register Apart from a reduction in readability there are two main reasons why I'd avoid it:

  1. If you use a class then you dont need it anyway - only records 'seem' to benefit from it.
  2. Using the debugger to follow the code to the declaration with Ctrl-Enter doesnt work.

That said, for readability I still use the syntax:

procedure ActionOnUpdate( Sender : TObject )
begin
  With Sender as TAction do
    Enabled := Something
end;

I've not seen a better construct.

Brian Frost
+5  A: 

I tend towards baning the with-statement altogether. As previously stated, it can make things complicated, and my experience is that it will. Many times the debugger want evaluate values because of withs, and all to often I find nested withs that lead to code that hard to read.

Brian's code seems readable and nice, but the code would be shorter if you just typecast the sender directly, and you remove all doubt about that component you enable:

TAction(Sender).Enabled := Something;

If you are concerned about typing to much, I prefare to make a temporary referance to the long-named object:

var
  t: TTable;
begin
  t := theLongNamedDataModule.WithItsLongNamedTable;
  t.FieldByName(' ');
end;

I can't se why typing should bother you, though. We Are Typists First, Programmers Second, and code-completion, copy-paste and key-recording can help you be a more effective typist.

update: Just stumbled over an long article with a little section on with-statements: he with keyword. The most hideous, dangerous, blow-your-own-feet-off feature in the language. :-)

Vegar
I'm afraid (to be pedantic) it should be typecast as (Sender as TAction).Enabled := Somthing as this will perform runtime checking that Sender does descend from TAction and if not raise a nice error, a stright typecase will crash.
Toby Allen
But why should sender be anything other than TAction? If I use the same eventhandler for different kind of components, I will need a check for what kind of sender generated the event, but when I hook up an event to one spesific component, why should I expect anything else as the sender?
Vegar
Is not about typing, its about code clarity. Thanks for the feedback
Marioh
+8  A: 

The with keyword is a nice feature for making your code more readable but there are some pitfalls.

Debugging:

When using code like this:

with TMyClass.Create do
try
  Add('foo');
finally
  Free;
end;

There is no way to inspect the properties of this class, so always declare a variable and use the with keyword on that.

Interfaces:

When creating an interface in the with clause it lives till the end of your method:

procedure MemoryHog;
begin
  with GetInterfaceThatTakes50MBOfMemory do
    Whatever;
  ShowMessage('I''m still using 50MB of memory!');
end;

Clarity

When using a class in a with clause that has properties or method names that already exists within the scope, it can fool you easily.

with TMyForm.Create do
  Width := Width + 2; //which width in this with is width?

Of course when having duplicate names, you're using the properties and methods of the class declared in your with statement (TMyForm).

The_Fox
The 'with' construct confused the hell out of the debugger on Delphi 7. I have not used further versions of Delphi, and don't know if this bad behaviour has improved
Johan Buret
At least until BDS2006, it doesn't improved. D2007 and after, I don't know.
Fabricio Araujo
It doesn't work in D2007 either.
Pauk
+2  A: 

Your example, of a datamodule access within a button click, is a poorly contrived example in my opinion. The whole need for WITH goes away if you move this code into the data module where it should be. The OnClick then just calls LongNameDataModule.InsertStuff and there is no with needed.

With is a poor device, and you should look at your code to see why you are needing it. You probably did something wrong, or could do it a better way.

mj2008
+9  A: 

The biggest danger of with, outside of pathological conditions like "with A, B, C, D" is that your code can silently change meaning with no notice to you. Consider this example:

with TFoo.Create
try
  Bar := Baz;
  DoSomething();
finally
  Free;
end;

You write this code knowing that Bar is a property of TFoo, and Baz is a property of the type containing the method which has this code.

Now, two years later, some well-meaning developer comes in adds a Baz property to TFoo. Your code has silently changed meaning. The compiler won't complain, but the code is now broken.

Craig Stuntz
Brrr... That one is really terrifying.
Fabricio Araujo
+1  A: 

There are many excelent answers here as to why the with statement is bad, so I'll try not to repeat them. I've been using the with statement for years and I'm very much starting to shy away from it. This is partially beause it can be difficult to work out scope, but I've been starting to get into refactoring lately, and none of the automated refactorings work withing a with statement - and automated refactoring is awesome.

Also some time ago I made a video on why the with statement is bad, it's not one of my best works but Here it is

Alister
A: 

I'm a firm believer of removing WITH support in Delphi. Your example usage of using a datamodule with named fields is about the only instance I could see it working out. Otherwise the best argument against it was given by Craig Stuntz - which I voted up.

I just like to point out that over time you may eventually (should) rmeove all coding in OnClick events and your code will also eventually migrate away from named fields on datamodules into using classes that wrap this data and the reason to use WITH will go away.

Darian Miller
+1  A: 

Your question is an excellent example of 'a hammer is not always the solution'.

In this case, 'with' is not your solution: You should move this business logic out of your form into your datamodule. Not doing so violates Law of Demeter like mghie (Michael Hieke) already commented.

Maybe your example was just illustrative, but if you are actually using code like that in your projects, this is what you should do in stead:

procedure TLongNameDataModule.AddToLongNameTable1(const NewField1Value: string);
begin  
  LongNameTable1.Insert;
  LongNameTable1_Field1.Value := NewField1Value;
  LongNameTable1_Field2.Value := LongNameTable2_LongNameField1.Value;
  LongNameTable1_Field3.Value := LongNameTable3_LongNameField1.Value;
  LongNameTable1_Field4.Value := LongNameTable4_LongNameField1.Value;
  LongNameTable1.Post;
end;

And then call it from your form like this:

procedure TMyForm.AddButtonClick(Sender: TObject);
begin  
  LongNameDataModule.AddToLongNameTable1('some value');
end;

This effectively gets rid of your with statement, and makes your code more maintainable at the same time.

Of course surrounding Delphi strings with single quotes will help making it compile as well ;-)

--jeroen

Jeroen Pluimers