関数がクラスのインスタンスを返してくる場合

解決


かえで  2004-03-28 22:11:11  No: 7986  IP: [192.*.*.*]

すみません、また初歩的な質問させてください。
関数がクラスのインスタンスを返してくる場合、関数の呼び出し側では
このインスタンスをどう扱えばよいのでしょうか?
例えば、

function DoSomething(Bitmap: TBitmap): TBitmap;

のように、引数に与えられたビットマップに何らかの処理をして
それを戻り値で返してくるような関数があったとすると、

procedure TForm1.Button1Click(Sender: TObject);
var
  Bitmap: TBitmap;
begin
  Bitmap := TBitmap.Create;
  try
    Bitmap := DoSomething(Image1.Picture.Bitmap);
    Bitmap.SaveToFile(ExtractFilePath(ParamStr(0)) + 'Result.bmp');
  finally
    Bitmap.Free;
  end;
end;

こんなことしても大丈夫でしょうか?
それとも、この場合は代入しているからOKで、
    Bitmap.Assign(DoSomething(Bitmap));
みたいにするとメモリリークしちゃうのですか?

編集    削除
るるとん@K  2004-03-28 22:30:18  No: 7987  IP: [192.*.*.*]

>Bitmap := TBitmap.Create;
代入するなら不要なのでは

編集    削除
るるとん@K  2004-03-28 22:33:20  No: 7988  IP: [192.*.*.*]

DoSomethingがひきすうを操作しひきすうを返すのならfreeはいりません(多分
DoSomethingの処理によります

編集    削除
かえで  2004-03-28 23:06:26  No: 7989  IP: [192.*.*.*]

> >Bitmap := TBitmap.Create;
> 代入するなら不要なのでは
確かにそうですね。(^_^;)
訂正します。

procedure TForm1.Button1Click(Sender: TObject);
var
  Bitmap: TBitmap;
begin
  Bitmap := DoSomething(Image1.Picture.Bitmap);
  try
    Bitmap.SaveToFile(ExtractFilePath(ParamStr(0)) + 'Result.bmp');
  finally
    Bitmap.Free;
  end;
end;

編集    削除
たかみちえ  URL  2004-03-28 23:21:12  No: 7990  IP: [192.*.*.*]

インスタンスというものについて、よく考えてみてください。
インスタンスは実体で、
TBitmap.Create;は、TBitmapというクラスをインスタンス化するための文です。
実体となったデータの所在地を覚え、参照するために、
Bitmap: TBitmap; という変数は存在します。  ここまではわかりますね。

  DoSomethingは、TBitmapを返す関数になっていますが、
これは先ほどまで使っていた(おそらく引数に指定した)Bitmapへの参照(ポインタ)を返します。
…ということは、引数に指定したビットマップへの参照を、DoSomething関数が返してBitmapに代入してしまった場合、
最初に生成したTBitmapのインスタンスは行方不明になってしまい、解放することができなくなってしまいます。

  このような場合は、関数が戻り値として、hBitmapを返す(このサイトにあるサンプル参照)などするのがいいのでしょうが、
この場合はPicture1はまた別の独立したオブジェクトになってますので(ReleaseHandleするわけにはいかない)、
Drawメソッドなどでコピーさせるといいでしょう。
  少し危なっかしいかもしれませんが、この場合
Bitmap.Canvas.Draw(0,0,DoSomething(Image1.Picture.Bitmap));
とすることで何とかできると思います。

  そして最後に、Bitmap.Assign(DoSomething(Bitmap));ですが、
これはもうわかりますね、わたしの修正案のとおりならば、代入されているのはあくまで最初に生成したものなので必要です。
引数に指定したものの場合は、いつもどおりVCLオブジェクトが解放するので不要です(やってはいけません)
関数で作った全く別のものを返した場合は、前者と扱い的に同じになるので必要です。

  私見ですが、
procedure DoSomething(Bitmap: TBitmap;var Dest: TBitmap);
として渡された変数Destを使って"戻り先として使うビットマップに、ビットマップをコピーする"ということをした方がいいかもしれませんね。
この方が、ぱっと見"どのようなBitmapを必要としてる"のか、わかりやすいと思います。
(好みだと思いますけど)

編集    削除
たかみちえ  URL  2004-03-28 23:28:25  No: 7991  IP: [192.*.*.*]

最後に の部分で言っているのは、Bitmap.Assign(DoSomething(Bitmap));ではなく、Bitmap.Free;のことですね(^^ゞ
返されるのは、普通の変数のようにデータそのものではなく、
あくまで"参照である"ということに気をつけてください。
  何でこんなことをするのかとも言われそうな気もしますけど、まあ、このほうが便利なケースというのも、多々あると思いますよ(^^ゞ

編集    削除
るるとん@K  2004-03-29 00:06:47  No: 7992  IP: [192.*.*.*]

DoSomethingが引数であるImage1.Picture.Bitmapを返すなら、
DoSomething(Image1.Picture.Bitmap).SaveToFile(ExtractFilePath(ParamStr(0)) + 'Result.bmp');
だけでいいです(多文
DoSomethingが新しく作成したTBitmapを返すならBitmapにDoSomething(Image1.Picture.Bitmap)を代入し、保存し、解放です。

編集    削除
るるとん@K  2004-03-29 00:11:48  No: 7993  IP: [192.*.*.*]

普通の変数を普通に引数にしたら、そのコピーが関数で使われますが、
クラスのインスタンスはポインタみたいになります。

var
A,B:TStringList;
begin
A:=TStringList.Create;
B:=A;
A.Text:='てすと'
//この時B.Textは「てすと」です(多分
end;

編集    削除
たかみちえ  URL  2004-03-29 00:37:12  No: 7994  IP: [192.*.*.*]

> 多文
  ここでの回答は、正確とは限らないものですが、
自分の中で「多分」なものばかりを投稿するのはどうかと思いますが…。

  回答するのならばそれなりに"確実"と思う回答をした方がいいと思います。
"自分の最低限答えられるもの以外には一切口を挟まない"というのも、少し臆病な考え方かもしれませんが、
あまりあってるかどうかすらわからないことばかりだと、質問した人も不安に思ってしまいますよ。
# わたしもちょっと前まではそんなことをしていましたから、人のことは言えませんが。

  DoSomething関数が返すのはポインタですから、
> DoSomething(Image1.Picture.Bitmap).SaveToFile(ExtractFilePath(ParamStr(0)) + 'Result.bmp');
みたいなことは可能です。
そういえば今気がつきましたが、ParamStr(0)は、自分のアプリケーションのフルパスです。
(たとえばDelphiならH:\Delphi7\Bin\Delphi32.exe など)
アプリケーションと同じフォルダに保存したいだけの場合は、
ExtractFileDirを使うといいですよ(IncludeTrailingPathDelimiterと併用するとよいかも)。


> ポインタみたい
  ポインタです(^^ゞ ポインタでないように扱えるようにはなっていますが。
ヘルプの"クラスとオブジェクト"のページを見てください。

編集    削除
かえで  2004-03-29 04:18:24  No: 7995  IP: [192.*.*.*]

たかみちえさん、るるとん@Kさん、申し訳ありません。
私が提示したDoSomething関数の仕様が曖昧でした。
次のように訂正させていただきます。

function DoSomething(const Src: TBitmap): TBitmap;
var
  Dest: TBitmap;
begin
  Assert(Assigned(Src));
  Dest := TBitmap.Create;

  { Srcのデータを使用してDestを加工 }

  Result := Dest;
end;

戻り値をTBitmap型の変数に代入して、使用後に解放する
ということでよろしいでしょうか?

> 私見ですが、
> procedure DoSomething(Bitmap: TBitmap;var Dest: TBitmap);
> として渡された変数Destを使って"戻り先として使うビットマップに、
> ビットマップをコピーする"ということをした方がいいかもしれませんね。
> この方が、ぱっと見"どのようなBitmapを必要としてる"のか、わかりやすいと思います。
> (好みだと思いますけど)
おっしゃるとおりです。
その方が使用する側も迷わないです。
ただ、他の人が作った関数なので。(^_^;)

> そういえば今気がつきましたが、ParamStr(0)は、自分のアプリケーションのフルパスです。
> (たとえばDelphiならH:\Delphi7\Bin\Delphi32.exe など)
> アプリケーションと同じフォルダに保存したいだけの場合は、
> ExtractFileDirを使うといいですよ(IncludeTrailingPathDelimiterと併用するとよいかも)。
すみません、これはちょっと意味がわかりませんでした。

> ParamStr(0)は、自分のアプリケーションのフルパス
これはわかります。

> アプリケーションと同じフォルダに保存したいだけの場合は、
> ExtractFileDirを使うといいですよ(IncludeTrailingPathDelimiterと併用するとよいかも)。
ExtractFilePathでなく、ExtractFileDir + IncludeTrailingPathDelimiter
を使う方がよいのはなぜでしょうか?

編集    削除
るるとん@K  2004-03-29 04:45:21  No: 7996  IP: [192.*.*.*]

procedure TForm1.Button1Click(Sender:TObject);
var
Bitmap:TBitmap;
begin
Bitmap:=DoSomething(Image1.Picture.Bitmap);
try
Bitmap.SaveToFile(ExtractFilePath(application.exename)+'Result.bmp');
finally
Bitmap.Free;
end;

編集    削除
たかみちえ  URL  2004-03-29 05:20:56  No: 7997  IP: [192.*.*.*]

>戻り値をTBitmap型の変数に代入して、使用後に解放する
>ということでよろしいでしょうか?

>ただ、他の人が作った関数なので。(^_^;)
  なるほど、TBitmapを作って返すのですね(^^ゞ
ならばそれでいいでしょう。


  ちなみに、別名関数を作ってラッピングするという手もありますよ、なんちゃって(^^ゞ
procedure DoImageConfig(Source: TBitmap; var Dest: TBitmap);
begin
  Dest := DoSomething(Source);
end:
まあ、そんなことしなくてもいいでしょう(^^ゞこの行は忘れてください。

>> アプリケーションと同じフォルダに保存したいだけの場合は、
>> ExtractFileDirを使うといいですよ(IncludeTrailingPathDelimiterと併用するとよいかも)。
>ExtractFilePathでなく、ExtractFileDir + IncludeTrailingPathDelimiter
  あ、ごめんなさい、ExtractFileDir のことでした。ExtractFilePathは、大丈夫です。
うーん、上でああいっただけに形無し…、ごめんなさいm(__)m

編集    削除
かえで  2004-03-29 05:36:26  No: 7998  IP: [192.*.*.*]

たかみちえさん、るるとん@Kさん、どうもありがとうございました。
おかげさまで解決できました。
今回のスレッドで、インスタンスを返す関数は使いにくいと痛感しました。
生成は呼び出した側でした方が見通しがよさそうですね。

編集    削除
sadoyama  URL  2004-03-29 09:53:16  No: 7999  IP: [192.*.*.*]

投稿者が「解決」としているのを蒸し返して申し訳ありませんが。

メモリーリークに関して言えば、問題は
function DoSomething(Bitmap: TBitmap): TBitmap;
にあります。

示されたところによると、
function DoSomething(const Src: TBitmap): TBitmap;
var
  Dest: TBitmap;
begin
  Assert(Assigned(Src));
  Dest := TBitmap.Create;

  { Srcのデータを使用してDestを加工 }

  Result := Dest;
end;
という内容です。

CreateされたDestがどこでも開放されないままこの関数を抜けてしまいますので、メモリーリークになります。
アプリそのものを終了する際には開放されますので、100個くらいのBitmapを処理するぶんには不都合は起きないでしょうが。

>こんなことしても大丈夫でしょうか?
>それとも、この場合は代入しているからOKで、
>    Bitmap.Assign(DoSomething(Bitmap));
>みたいにするとメモリリークしちゃうのですか?
この質問への直接の答えにはなっていませんが、いずれにせよ、上記の問題が解決しないといかなる質問も有効とはならいと考えます。


ではどうすれば良いかですが、面倒なので突き詰めて調べるに至っていません。
ただ、私が十分な使用を繰り返していて、実践的には不都合が起きていない方法を紹介しておきます。
正しい処理だという理論的確信はありませんが、たかみちえ さんと同じ方式のようです。

たかみちえ さん wrote
>procedure DoSomething(Bitmap: TBitmap;var Dest: TBitmap);
>として渡された変数Destを使って"戻り先として使うビットマップに、ビットマップをコピーする"ということをした方がいいかもしれませんね。
>この方が、ぱっと見"どのようなBitmapを必要としてる"のか、わかりやすいと思います。

かえで さん wrote
>生成は呼び出した側でした方が見通しがよさそうですね。
以下のコードはたぶんこれと同じ考え方でしょう。


procedure DoSomething(Src, Dest: TBitmap);// procedureに変更
begin        // Object引数につき const, varは不要
  Assert(Assigned(Src));
  Assert(Assigned(Dest));

  { Srcのデータを使用してDestを加工 }
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  DestBmp: TBitmap;
begin
  Image1.Picture.Bitmapに画像をセット

  DestBmp := TBitmap.Create;
  try
    DoSomething(Image1.Picture.Bitmap, DestBmp);
    DestBmp.SaveToFile(????? + 'Result.bmp');
  finally
    DestBmp.Free;
  end;
end;

編集    削除
つっか  2004-03-29 12:02:57  No: 8000  IP: [192.*.*.*]

> CreateされたDestがどこでも開放されないままこの関数を抜けてしまいますので、メモリーリークになります。

関数やメソッドがクラスのインスタンスを返すものは VCL にもけっこうありますよ。
たとえば TForm が TCustomForm から継承している GetFormImage; という
メソッドは TBitmap のインスタンスを返しますが、使用後の破棄は使用者の
責任になっています。

編集    削除
take  2004-03-29 17:48:54  No: 8001  IP: [192.*.*.*]

そのfunction DoSomething(const Src: TBitmap): TBitmapというのが
何度も実行されるのであれば
クラスの生成時に FSomethingBitmap : TBitmap;
とビットマップをあらかじめ生成しておきDoSomethingの中で

Result := FSomethingBitmap;

と渡すのが無難でしょう

もちろんDoSomethingを実装しているクラスの破棄イベントで
FSomethingBitmapを解放する必要があります。
元のクラスを書き換えられないならoverrideなどしてでも
確実に作っておいた方が良いです。

渡される側で破棄するという方法で今回作られたとしても
もし別のプロジェクトで使用されるときに混乱が生じるでしょう。

それと適当に書かれた文章やソースが見あたりますが、
メモリリークは直接プログラムの暴走につながることですので
慎重に議論する必要があると思います。
特にるるとん@Kさんの適当な回答が目立ちます。
長期間放置されている質問なら別ですが、答えられない質問に
無理して見当違いの回答をしてもしかたないでしょう。

編集    削除
つっか  2004-03-29 18:11:37  No: 8002  IP: [192.*.*.*]

>take さん
>クラスの生成時に FSomethingBitmap : TBitmap;

かえでさんの書き込みにありますように、DoSomething はクラスのメソッドでは
なく独立した関数です。また、関数がその外の変数を参照するのは完結しないこと
のほかに、やはり廃棄の問題があります。

>渡される側で破棄するという方法で今回作られたとしても
>もし別のプロジェクトで使用されるときに混乱が生じるでしょう。

そうですね。インスタンスの生成はできるだけコード上で距離の短いところで
行われるべきですね。

> 特にるるとん@Kさんの適当な回答が目立ちます。

もう、前から何度も注意してますが変わりません。質問に直接関係ないこと、
間違ったこと、ちゃちゃを書き込むので、質問者がおそれをなして[解決]マーク
を付けられないスレッドもいくつかあるようです。わたしはあきらめています。

編集    削除
にしの  2004-03-29 18:27:32  No: 8003  IP: [192.*.*.*]

> かえでさんの書き込みにありますように、DoSomething はクラスのメソッドでは
> なく独立した関数です。また、関数がその外の変数を参照するのは完結しないこと
> のほかに、やはり廃棄の問題があります。

Singletonを実装したいのであれば、独立した関数でも可能ですよ。
ただし、関数を別のユニットに書かないとちょっと煩雑になります。

unit uTest;

uses
  Graphics; // 他に必要であればuses節に加える

function DoSomething: TBitmap;

implementation

var
  FBitmap: TBitmap;

function DoSomething: TBitmap;
begin
  if not Assigned(FBitmap) then FBitmap:=TBitmap.Create;
  Result := FBitmap;
end;

initialization
  FBitmap := nil;
finalization
  if Assigned(FBitmap) then FBitmap.Free;

end.

こんな感じ。

編集    削除
jok  2004-03-29 18:36:30  No: 8004  IP: [192.*.*.*]

> にしのさん
> Singletonを実装したいのであれば、独立した関数でも可能ですよ。

そこまで大がかりにすることはないのでは。関数としての独立性もなくなって
ユニット依存になってしまいます。

> つっかさん
> わたしはあきらめています。

いや、書き込むこと自体は問題ないのでは?彼のほかにも質問から遊離した回答
はよくあることです。他人のQ&Aを見ていて疑問を感じたりすることはよくあ
ります。せめて、別スレッドをたてて質問してくれたらいいと思うんですが。
最初のほうからくらべると、彼の成長が分かってきてけっこう楽しみにしています。

編集    削除
take  2004-03-29 18:42:50  No: 8005  IP: [192.*.*.*]

まとめると

function DoSomething(const Src: TBitmap): TBitmap;
var
  Dest: TBitmap;
begin
  Assert(Assigned(Src));
  Dest := TBitmap.Create;

  { Srcのデータを使用してDestを加工 }

  Result := Dest;
end;

という(他人)が作ったプログラムをメモリリークしないように
処理するには?ということで最終的には

procedure TForm1.Button1Click(Sender: TObject);
var
  Bitmap: TBitmap;
begin
  Bitmap := DoSomething(Image1.Picture.Bitmap);
  try
    Bitmap.SaveToFile(ExtractFilePath(Application.ExeName) + 'Result.bmp');
  finally
    // DoSomething内で Bitmapが生成されているので、ここで破棄させる
    Bitmap.Free;
  end;
end;

で解決したわけですね。
この書き方ならリークはありませんが、他の方法でDoSomethingが呼ばれると
問題があるかもしれないのでFunction化などして、どこか1カ所で管理した
ほうが良いでしょう。
時間が経ってからこのソースを見てどうしてこんな書き方しているのかな?
ということがないようにしっかりコメントをつけておくことをお奨めします。

編集    削除
jok  2004-03-29 19:01:00  No: 8006  IP: [192.*.*.*]

ちょっとしつこいかも知れませんが、なんだか議論が堂々巡りになっています。
インスタンスは生成したら破棄する、というのは当たり前です。クラスのインスタンス
を返すのは、コンストラクタでも同じですね。関数の場合は、Free を忘れやすい
というだけのことではないですか。程度の問題です。使う方が責任をもつ、VCL と
同じスタンスでよいと思います。

編集    削除