Funkcja zwracająca klasę np TGraphic, a zwalnianie pamięci.

Jak rozwiązać problem ze zwalnianiem pamięci jeśli funkcja zwraca jakąś klasę. Przedstawie problem na przykładzie TGraphic;

function Test(AStream: TMemoryStream): TGraphic;
var
  Pic: TRafPic;
begin
  Pic := TRafPic.Create(AStream); //AStream zostanie rozpoznane jako jpg/bmp/tiff/gif/png itp
  Result := Pic;
  //Pic.Free;  Tego nie mogę wykonać bo funkcja zwróci błąd  ale jeśli tego nie zrobię to PIC nie zostaje zwolnione z pamięci i to pokazuje mi FastMM4
end;

PROGRAM

  Logo.Picture.Graphic := test(AStream);

Jakieś pomysły jak to obejść? Czy to w ogóle możliwe?

1. Żadnych Rafik'ow.
2. Wywal niepotrzebne zmienne
3. Pozostaje jeden wiersz:

function Test(AStream:TMemoryStream):TGraphic;
begin
  Result:=TGraphic.Create(AStream); //AStream zostanie rozpoznane jako jpg/bmp/tiff/gif/png itp
end;

więc cała funkcja bez sensu.

Teraz sam problem:

var Pic:TGraphic;

Pic:=TGraphic.Create(AStream);
try
  Logo.Picture.Assign(Pic);
finally
  Pic.Free;
end;

Z czego można zrobić procedurę:

procedure LoadPic(Picture:TPicture;AStream:TMemoryStream)
var Pic:TGraphic;
begin
  Pic:=TGraphic.Create(AStream);
  try
    Picture.Assign(Pic);
  finally
    Pic.Free;
  end;
end;

Dorzucę swoje 3 grosze...
Nigdy, ale to nigdy nie powinno się pisać funkcji, które zwracają w Result instancję obiektu. To oczywiste proszenie się o kłopoty, bo taka funkcja wprowadza zamieszanie. Co w przypadku, kiedy funkcja nie może zwrócić instancji obiektu (bo cokolwiek)? Powinna zwrócić nil?
Poza tym - kto ma zniszczyć ten obiekt? Bo zapis jak podano wyżej sugeruje, że to funkcja zarządza cyklem życia obiektu - a tak nie jest.
Można napisać np. taki kod:

Test.Transparent := True;

I wszystko jest teoretycznie OK, poza gwarantowanym wyciekiem pamięci.
Dlatego też, zgodnie z zasadami programowania defensywnego, powinno to wyglądać np. tak:

function Test(AStream : TMemoryStream; var AGraphic : TGraphic) : Boolean;
begin
  try
    if Assigned(AStream) then
    begin
      AGraphic := TRafPic.Create(AStream); //AStream zostanie rozpoznane jako jpg/bmp/tiff/gif/png itp
      Result := True;
    end
    else
      Result := False;
  except
    on E: Exception do
    begin
      Result := False;
      raise;
    end;
  end;
end;

I samo użycie:

var
  lGraphic : TGraphic;
begin
  if Test(lStream, lGraphic) then
    lGraphic // coś tam
end;

Dlaczego tak jest lepiej? Ponieważ:

1. Programista musi zadeklarować zmienną do której dostanie efekt działa funkcji. Co przy funkcji, która zwraca instancję obiektu, nie jest wymogiem.
2. Jak programista sam zadeklaruje zmienną, to od razu widzi, że musi ją sam zwolnić.
3. Ma pewność, czy wywołanie funkcji się powiodło - wystarczy sprawdzić czy Result = True

Dziękuję za odpowiedź. Takie proste do zrobienia a tak trudno czasami na to wpaść :)

Gotowa procedura wygląda tak i działa bez wycieków.

procedure LoadImageFromBlob(MyData: TMyQuery; Field: String; Picture: TPicture);
var
  AStream: TMemoryStream;
  Pic: TRafPic;
begin
  AStream := TMemoryStream.Create;
  Try
    (MyData.FieldByName(Field) as TBlobField).SaveToStream(AStream);
    Try
      Pic := TRafPic.Create;
      Pic.LoadFromStream(AStream);
      Picture.Assign(Pic);
    Finally
      Pic.Free;
    End;
  Finally
    AStream.Free;
  End;
end;

wloochacz napisał(a):

Dorzucę swoje 3 grosze...
Nigdy, ale to nigdy nie powinno się pisać funkcji, które zwracają w Result instancję obiektu. To oczywiste proszenie się o kłopoty, bo taka funkcja wprowadza zamieszanie.

Dobrze, że wzorzec metody fabrykującej o tym nie wie! To by się GoF zmarwił...