[delphi] input uit ander form halen

[tharkun]

Het is de bedoeling dat de text in editbox1 tm 5 in de paralist wordt gestopt. editboxX is van type Tedit.
Maar om een of andere reden neemt delphi niet de tekst die wordt ingetypt maar de tekst die in de desinger is opgegeven.
Als ik er handmatig een string instop gaat het wel goed (paralist.Add('test');.
Wat mist hier?

code:

function TAdd.getParameters():TStringList;
var
  paralist: TStringList;
begin
  paralist:= TStringList.create;

  paralist.Add(editbox1.text);
  paralist.Add(editbox2.text);
  paralist.Add(editbox3.text);
  paralist.Add(editbox4.text);
  paralist.Add(editbox5.text);

  Result := paralist;
end;

procedure TMainform.new_agent;
var
  add_dialog: TForm;
  Para_list: TStringlist;
begin
  add_dialog := Tadd.Create(nil);
  Para_list := TStringList.create;
  if add_dialog.ShowModal = mrOk then
    begin
      
      Para_list := add_dialog.getParameters;
      ShowMessage(Para_list.CommaText);
    end
  else
    begin
      // 
    end;
   add_dialog.Release;
end;

[Verwijderd]

tharkun schreef op 15 september 2004 @ 14:48:

om te beginnen moet dit: add_dialog: TForm;
ff omgezet worden naar: add_dialog: TAdd;

vermoed ik.

En je bent sowieso geheugenlekken aan 't creeren,
wie free'd de para_list ?

[ Voor 91% gewijzigd door Verwijderd op 15-09-2004 14:56 ]

[tharkun]

geheugenlekken: klopt. Maar ik wil eerst iets in het geheugen kunen stoppen voordat ik het er weer uithaal.

Omzetten naar TAdd. Dat helpt niet. TAdd = class(TForm)

[Verwijderd]

tharkun schreef op 15 september 2004 @ 15:05:
geheugenlekken: klopt. Maar ik wil eerst iets in het geheugen kunen stoppen voordat ik het er weer uithaal.

Omzetten naar TAdd. Dat helpt niet. TAdd = class(TForm)

Kijk eens wat er staat als je ipv:
ShowMessage(Para_list.CommaText);
-> ShowMessage(add_dialog.editbox1.text);
zet.

EN je kan beter dit doen (ff afgezien van een aantal opmerkingen van TomotaMan -zie onder - :

code:

procedure TAdd.getParameters(paraList :TStringList);
begin
 if not assigned(paralist) then
     paralist:= TStringList.create
 else
    paralist.clear;

  paralist.Add(editbox1.text);
  paralist.Add(editbox2.text);
  paralist.Add(editbox3.text);
  paralist.Add(editbox4.text);
  paralist.Add(editbox5.text);

end;

procedure TMainform.new_agent;
var
  add_dialog: TForm;
  Para_list: TStringlist;
begin
  add_dialog := Tadd.Create(nil);
  Para_list := TStringList.create;
  if add_dialog.ShowModal = mrOk then
    begin
      
      add_dialog.getParameters(para_list);
      ShowMessage(Para_list.CommaText);
    end
  else
    begin
      // 
    end;
   para_list.free;
  add_dialog.free;
end;

[ Voor 6% gewijzigd door Verwijderd op 15-09-2004 15:27 ]

[Tomatoman]

Om te beginnen raad ik je aan wat consistentere naamgeving te gebruiken. In Delphi is het gebruikelijk de namen van variabelen, functies en procedures te starten met een hoofdletter en in plaats van een underscore een nieuwe hoofdletter te gebruiken. Dus: AddDialog in plaats van add_dialog, ParaList in plaats van Para_list en GetParameters in plaats van getParameters. Dit maakt je code beter leesbaar.

Er zitten diverse bugs in je code. Zo creëer je in regel 22 een TStringList die je in de variabele Para_list stopt. In regel 26 ken je aan Para_list een ander TStringList toe, waardoor je geen referentie meer hebt naar het object dat je in regel 22 hebt gecreëerd. Het object uit regel 22 wordt daardoor nooit meer vrijgegeven, terwijl het object dat in regel 5 wordt gecreëerd in regel 33 wordt vrijgegeven.

Overigens (dat is een andere bug) dien je dat object vrij te geven middels Free in plaats van Release. Lees de helpfiles er maar op na.

tharkun schreef op 15 september 2004 @ 15:05:
geheugenlekken: klopt. Maar ik wil eerst iets in het geheugen kunen stoppen voordat ik het er weer uithaal.

Dit is een typisch geval van slecht programmeren. Geheugenlekken tijdelijk laten zitten omdat het je even niet uitkomt wat extra code te schrijven is zeer gevaarlijk. De kans is groot dat je vergeet dat er een geheugenlek is en dat de fout voor altijd in de code blijft zitten. De grondregel is dat je een tijdelijk object ALTIJD door middel van een try...finally blok weer vrijgeeft.

[Tomatoman]

Aangezien de code een beetje een puinhoop is (sorry, zie het maar als een leermoment ), heb ik het even omgeschreven naar bugvrij en nette code. Het zal je opvallen dat objecten in de procedure waar ze worden gecreëerd ook weer worden vrijgegeven (dus niet verspreid over twee procedures/functies). Dat maakt de code duidelijker en voorkomt bugs.

Delphi:

function TAdd.GetParameters(Strings: TStrings);
begin
  with Strings do
  begin
    { Clear is niet noodzakelijk als je de procedure aanroept
      vanuit NewAgent, maar wel als je de procedure algemeen
      bruikbaar wilt houden. Kwestie van netjes programmeren. }
    Clear;
    Add(EditBox1.Text);
    Add(EditBox2.Text);
    Add(EditBox3.Text);
    Add(EditBox4.Text);
    Add(EditBox5.Text);
  end;
end;

procedure TMainform.NewAgent;
var
  AddDialog: TAdd;
  ParaList: TStrings; // of TStringList
begin
  AddDialog := TAdd.Create(nil);
  try
    ParaList := TStringList.Create; { niet TStrings, dat is een
                                      abstracte klasse! }
    try
      if AddDialog.ShowModal = mrOk then
      begin
        AddDialog.GetParameters(ParaList);
        ShowMessage(ParaList.CommaText); // niet ParaList.Text???
      end
      else
      begin
        //
      end;
    finally
      ParaList.Free;
    end;
  finally
    AddDialog.Free;
  end;
end;

[tharkun]

van
add_dialog.getParameters(para_list);
heb ik dit gemaakt
Para_list := add_dialog.getParameters();

Als ik niet dit doe
if not assigned(paralist) then
paralist:= TStringList.create
else
paralist.clear

maar gewoon paralist := TStringlist.create

dan werkt het!

[Tomatoman]

tharkun schreef op 15 september 2004 @ 15:39:
van
add_dialog.getParameters(para_list);
heb ik dit gemaakt
Para_list := add_dialog.getParameters();

Als ik niet dit doe
if not assigned(paralist) then
paralist:= TStringList.create
else
paralist.clear

maar gewoon paralist := TStringlist.create

dan werkt het!

En dan heb je nog steeds een geheugenlek. Begrijp je eigenlijk wel wat je eigen code precies doet? Volgens mij niet.