OpenRTM / OpenRTM-aist

OpenRTM-aist: RT-Middleware and OMG RTC implementation in C++ implemented by AIST
https://openrtm.org/
Other
19 stars 12 forks source link

Factoryの実装見直し #560

Closed Nobu19800 closed 3 years ago

Nobu19800 commented 5 years ago

Is your feature request related to a problem? Please describe. Factoryクラスで生成したオブジェクトを内部の連想配列に追加、削除を行う処理があるが、作成済みのオブジェクトを取得するcreatedObjects関数を使用しているのはExecutionContextFactoryのみであり、ほかのファクトリについてはオブジェクトを配列に格納すること自体が無駄である。

Describe the solution you'd like 生成したオブジェクトの一覧が必要ないのであれば、自分で生成して削除するだけにしたい。

Describe alternatives you've considered .内部で配列にオブジェクトを格納する必要がある場合と必要がない場合があるため、2種類のFactoryを定義してExecutionContextFactoryとそれ以外で使い分けるべき。

Additional context

Nobu19800 commented 5 years ago

@n-ando

coil::FactoryのDestructorでdynamic_castしている意図が不明です。

https://github.com/OpenRTM/OpenRTM-aist/blob/bc6ef143a448deaa82ce5bffdaad233d31c80196/src/lib/coil/common/coil/Factory.h#L107-L115

またそもそもFactoryでデストラクタを指定する必要自体があるでしょうか?

https://github.com/OpenRTM/OpenRTM-aist/blob/bc6ef143a448deaa82ce5bffdaad233d31c80196/src/lib/coil/common/coil/Factory.h#L242-L244

n-ando commented 3 years ago

モジュール境界を考慮してこの様になっています。 外部DLLでnewしたオブジェクトは、そのDLL内のdeleteでデストラクトする必要があります。 大抵の場合はメインライブラリのdeleteでも問題は起きませんが、それはたまたまで、通常DLL内クラスのオブジェクトはDLL内のnewで生成し、DLL内のdeleteで削除する必要があります。

オブジェクトリストを保持しているのは、もしDLLをアンロードする際には、そのDLLのオブジェクトをすべて削除する必要があるためです。実装はしていませんが、あるDLL・クラスのオブジェクトをFactory で管理しておき、インスタンス数が0であればDLLをアンロードすることができます。 そのような理由でこのような構造になっています。

以上のような理由から、現状のFactoryの構造はそのままでいいです。