0

I have this code:

edit5.Text := IntToStr(j);
rw := j;
myRect.Left := 0;
myRect.Top := rw;
myRect.Right := 5;
myRect.Bottom := rw;
stringGrid1.Selection := myRect;
edit1.SetFocus;

I must rewrite this code because I'm using it for many events (event button1click, button2click, everytime I validate) so I'm meaning to make then into procedure so I can call it in many event this code so far I made:

procedure highlight(edit1, edit5: TEdit; myrect: TGridRect;
  stringgrid1: TStringgrid; var j, rw: Integer);
begin
  edit5.Text := IntToStr(j);
  rw := j;
  myRect.Left := 0;
  myRect.Top := rw;
  myRect.Right := 5;
  myRect.Bottom := rw;
  stringGrid1.Selection := myRect;
  edit1.SetFocus;
end;

but I can't call it:

procedure Tform1.Button2Click(Sender: TObject);
begin
  highlight;
end;

how to resolve it ? did I must split it ?

TLama
  • 75,147
  • 17
  • 214
  • 392
Chino
  • 23
  • 1
  • 4
  • Which book are you using that doesn't explain how to call procedures? We need to know that so we don't recommend it to anyone else. You should go find a better book. – Rob Kennedy Dec 23 '13 at 14:36

1 Answers1

1

Your extracted procedure is not quite right. You pass a rect that you don't use. You pass rw and j as var parameters, but it looks like a single by value parameter really. So I'd have it like this:

procedure Highlight(Edit1, Edit5: TEdit; StringGrid: TStringGrid; rw: Integer);
begin
  Edit5.Text := IntToStr(rw);
  StringGrid.Selection := Rect(0, rw, 5, rw);
  Edit1.SetFocus;
end;

Call it like this:

Highlight(Edit1, Edit5, StringGrid1, j);

Now, this assumes that you sometimes need to pass different controls to the procedure. If you always pass the same controls, then make the procedure be a method of your class:

procedure TMyForm.Highlight(rw: Integer);
begin
  Edit5.Text := IntToStr(rw);
  StringGrid.Selection := Rect(0, rw, 5, rw);
  Edit1.SetFocus;
end;

And call it like this:

Highlight(j);

I'm assuming here that the value you pass as j can vary. So it should be a parameter. That's the sort of reasoning that you need to use when deciding whether something should be a parameter, or use a field. Ask yourself if you will always pass the same value when calling the method?

Finally, you are making life hard by not naming your variables. How can a reader of the code know what is so special about Edit5 and why it is treated differently from Edit1. Give names to your variables.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • it works thank you with a few change stringGrid.Selection := TGridRect(rect(0, rw, 5, rw)); – Chino Dec 22 '13 at 22:53
  • That's a bad cast. I had not spotted it was TGridRect rather than TRect. Better off with a local variable of type TGridRect. Perhaps a helper function named GridRect. – David Heffernan Dec 22 '13 at 23:00
  • @DavidHeffernan Hmmm... it is pretty weird Embarcadero did not added helper functions or constructor or type casts themselves – Arioch 'The Dec 23 '13 at 13:35