views:

93

answers:

2

I have many methods in legacy code that use a TIBQuery (Interbase) with Unidirectional property = False. The problem is that users sometimes get out of memory exception. I suspect it could be fixed by setting this property to True as there is no need to cache the records.

Of course I don't want to broke the old code but I also want to fix the problem.

Here is a code sample (not complete because of size):

procedure TAnalyzeForm.CostByInvoice;
begin
  try
    qryReport.Close;
    qryReport.Open;
    qryReport.Last;
    qryReport.First;
    if qryReport.RecordCount > 0 then
    begin
      for i := 0 to qryReport.RecordCount - 1 do
      begin
        vInvoiceNo := Format('%-8s', [qryReport.FieldValues['InvoiceNo']]);
        vDeptId := Format('%8s', [qryReport.FieldValues['DepartmentId']]);
        vOrgName := Format('%-22s', [qryReport.FieldValues['OrgName']]);
        vInvDate := qryReport.FieldValues['InvoiceDate'];
        vInvNetCur := qryReport.FieldValues['InvNetCur'];
        vInvVatCur := qryReport.FieldValues['InvVatCur'];
        vInvTotCur := qryReport.FieldValues['InvTotCur'];
        vInvCur := qryReport.FieldValues['UnitId'];
        vTotNet := vTotNet + qryReport.FieldValues['InvNetValue'];
        vTotVat := vTotVat + qryReport.FieldValues['InvVatValue'];
        vTotTot := vTotTot + (qryReport.FieldValues['InvNetValue'] + qryReport.FieldValues['InvVatValue']);
        grdCost.Cells[1, i+1] := vInvoiceNo;
        grdCost.Cells[2, i+1] := vDeptId + ' ' + vOrgName;
        grdCost.Cells[3, i+1] := FormatDateTime('dd.mm.yyyy', vInvDate);
        grdCost.Cells[4, i+1] := Format('%12.2f', [vInvNetCur]);
        grdCost.Cells[5, i+1] := Format('%12.2f', [vInvVatCur]);
        grdCost.Cells[6, i+1] := Format('%12.2f', [vInvTotCur]);
        grdCost.Cells[7, i+1] := 'EUR';
        grdCost.RowCount := i+1 + 1;
        qryReport.next;
      end;
      txtNetCost.Caption := Format('%12.2f', [vTotNet]);
      txtVatCost.Caption := Format('%12.2f', [vTotVat]);
      txtTotCost.Caption := Format('%12.2f', [vTotTot]);
      SummaryInfo(stSummaryInfoCost, 'Number of costinvoices: ' + IntToStr(qryReport.RecordCount), time, true);
    end
    else
      MessageDlg('nothing found!', mtInformation, [mbOk], 0);
  finally
    qryReport.Close;
  end;
end;

The important variable is qryReport that is a TIBQuery. I want to rewrite it so it so I can set TIBQuery.Unidirectional = True. qryReport is reused at many places with different SQL so I think that is the reason for the Close, Open sequence in the beginning.

+6  A: 

Once you set Unidirectional to True, you can only call First and Next. That code has always been bad. You should never use RecordCount to iterate over records. Use Next and EOF. This way you have no need to call Last to force the database to load the whole result set just to get how many records there are.

ldsandon
Thanks for the answer! I will try that.
Roland Bengtsson
+1 I agree with Luigi
Fabricio Araujo
+1  A: 

If the same instance is used with many SQL queries and you main concern is not to break the application, when required, you can save the value of Unidirectional property, set it to True, use the object and restore it to its original value at the end.

Something like:

var
  OldUnnidirectionalValue: Boolean;
begin
  OldUnnidirectionalValue := qryReport.Unnidirectional;
  qryReport.Unnidirectional := True;
  try
    qryReport.SQL.Text := 'select what you want';
    qryReport.Open;
    try
      while not qryReport.eof do
      begin
        UseTheRecord;
        qryReport.Next;
      end;
    finally
      qryReport.Close;
    end;
  finally
    qryReport.Unnidirectional := OldUnnidirectionalValue;
  end;
end;

Avoid the Close Open sequence, you must use try/finally blocks to always close the query after usage.

Best regards ;)

jachguate
Looks reasonable, thanks for the example.
Roland Bengtsson