Menu
▾
▴
#101 CWSHIfObj(旧CInterfaceObject)の再利用可能性。
http://sakura-editor.sourceforge.net/cgi-bin/cyclamen/cyclamen.cgi?log=unicode&v=1182#1182
掲示板の件です。具体的に添付ファイルのようなものをイメージしていました。
ファイルをまたいだ移動のせいで差分のサイズが大きいですが実質的な変更はわずかです。
パッチに即して改めて説明しますと、旧CInterfaceObject相当のスリムな CWSHIfObjを継承した、
パッチ版の CEditorIfObjのようなオブジェクトを作りたかったのだけれども、そういう CWSHIfObjは
存在せず、パッチでいうところの CWSHIfObj_ImplementHelperを継承したオブジェクトしか作れない
状態だったのが困った、というわけです。
CEditView*に関しても virtual void CWSHIfObj::ReadyMethods( CEditView* pView, int flags )
という形で void*に頼らずに渡すようにしてありますが、CWSHIfObjにこういうメンバ関数を追加するのは、
一つだけだからいいかな、という妥協の結果ではないかと自分を疑っています。
CWSHIfObj_ImplementHelperがテンプレートである必要は全くないのですが
COutlineIfObjと CPluginIfObjが CWSHIfObjからも派生していることがちょっとでも
伝わるかなと思って一応ああいう形になっています。
JsOutline/outline.jsを自分自身でアウトライン解析できることは確認しています。
パッチを見ました。
質問ですが、CEditorIFObjは CWSHIfObjを継承しているのに対し、OutlineとPluginはCWSHIfObj_ImplementHelperを継承しているのは何ででしょう。例のためEditorはCWSHIfObjを継承しているが、最終的にImplementHelperを継承するようになるのですか？
もうひとつ質問ですが、InterfaceObjectをCWSHソースにまとめることは重要なのでしょうか。自分が昔のCWSHを見たとき、クラスが４つくらい入っていて構成を把握するのに苦労しました。見通しを良くするため InterfaceObjectは独立できそうだと思って切り出したのですが、もとに戻した方がよいですか？
まだパッチを全部把握してないですが、CWSHIfObj→CIfObj、～_ImplementHelper→CWSHIfObj とかにしたらわかりやすいんじゃないかと思いました。
反応ありがとうございます。
CEditorIfObjは _ImplementHelperを継承するつもりはありません。なくても十分簡潔に実装できていると思っていますので。COutlineIfObjも、m_MacroFuncInfoNotCommandArrと HandleFunction()が遊んでいますから _ImplementHelperを継承する必要はないんじゃないかと思っています。そのときには CEditorIfObj.cppにある ReadyCommands()のようなヘルパ関数が COutlineIfObj.cppにも必要でしょうから、ReadyCommands()を _ImplementHelperの静的関数にする手が考えられます。
ファイルをわけるなら CWSHClient+CWSHSiteと CWSHIfObj+CIfObjTypeInfoで分割できそうですね。以前は CWSHMacroManagerもいっしょくたでしたから、分割する余地がまだあっても不思議ではないです。問題があるとすれば自分が CWSHIfObjに追加した friend class CWSHClientです。これは CWSHIfObjのメンバ変数を publicから privateにしたときに CWSHClient::AddInterfaceObject()がアクセス違反をしたので短絡的に追加しました。Name()や IsGlobal()と同じようにアクセサを用意すれば済むのですが、メンバ変数一つ一つにアクセサを定義する作業は、言い知れぬ不安がわいてくるために 2つが限界でした。friendならいいかというと強力すぎてもっとダメなのですが、同じファイルに放り込んでいる限りは目をつぶって許される気がします(気のせいかも)。
名前ですが、Interfaceという語は必要だと思いますか？ スクリプトに公開されるオブジェクトだから CWSHObjectや CScriptObjectがわかりやすいのですが。
ちょっと思いついたのですが、syatさんは HandleCommand()や HandleFunctioin()をスクリプト以外の、C++コードに対するインターフェイスだと考えておられますか。ひょっとして既に利用していたりも。自分はスクリプトが呼ぶメソッドの実体だとしか見ていないので、そのシグネチャ(名前や引数)がオブジェクト(CEditorIfObjeや COutlineIfObj)ごとに異なっていたり、そもそも具えていなかったりすることを全然気にしていませんでした。そういう用途にも _ImplementHelperは使えると思いますが templateを削って名前を変えるのはもちろんのこと、アクセス制御やら他にも再考すべき点があるかもしれません。
「分割する余地がまだあっても不思議ではないです。」って、その分割を syatさんが既に行っていたのでした。CWSH.hにまとめたのには深い考えがあったわけではなく、古いリビジョンも参考にしながら CWSHIfObj.cppを分割したためにああなりました。手に負えなくなるまで分割を控えるのは自分の癖のようです。管理するファイルが少なく済み、依存関係が明白で、クラス定義が固まっていなくても許される気がするんだと思います。
率直な感想。スクリプトに渡すオブジェクトを作ろうと思ったときに、_ImplementHelperの方かそうじゃない方のどちらにするかで悩みたくないなぁ。それと、今COutlineIfObjをCWSHIfObj継承で作ったとして、機能追加で関数を提供したくなったときに親クラスを_ImplementHelperに変えるというのもどうか。という理由から、C????IfObjは共通の基底クラスを継承し、基底クラスのReadyCommandsを呼び出せるような構成を希望します。
その根底にある思いは、今後プラグインをいろいろなところに提供しようとするとWindowIfObjとかFoldIfObjとか何種類も作ることになりますが、WSHに関数を登録する部分は共通なので考えなくて良く、提供する関数のシグネチャと中身だけを記述すればよいようにしたいのです。
ですが今のCWSHIfObjはCWSHClientのデータにCEditView*を渡す前提で組んであり、ds14050さんの希望は満たせなかった、というふうに理解してます。
なので、もっとシンプルなInterfaceObject（仮にCIfObjBaseとしましょうか）を抜き出して、CWSHIfObjはそれを継承してReadyCommandsなどを追加する。シンプルなWSH機能を使いたい人はCIfObjBase、またはそれを継承した何かを使用する。そういう方向なら理解できますしとくに異論ありません。
名前についてですが、単にObjectだとActiveXObject？とかScriptControl？とか誤解されそうなので、"IfObj"ときたら「あぁ、WSHに渡すオブジェクトのことね」とピンと来るように刷り込みｗしていきたいです。
ちなみにC++コード（DLL）に対するインターフェースは私は一切考えていません。
＃誰か作ってくれたらいいなとは思っているけど、自分自身はそんなに必要としてないので。
＃.NETが使えるといいなぁ。Formが使えるから。
すみませんが私は明日から2日ほど家をはなれるので応答できなくなります。
trunk2@1755へのパッチ。
ヘルパであれ何であれ、共通の基底クラスを持つメリットもありますので、COutlineIfObjが _ImplementHelperを継承すべきでないとまでは思っていません。これが「スクリプト以外の、C++コードに対するインターフェイス」と書いた部分に関係することです。
例えば、COutlineIfObjは SetTitle()や SetListType()というメソッドをスクリプトに公開しています。同じ機能を HandleCommand()を通して C++コードから利用させるつもりがあるならば、プラグイン関連のクラスが共通の基底クラスを持つ動機になると思ったのです(例に挙げたメソッドはアウトラインに固有のメソッドっぽいので基底クラスの HandleCommand()を通して呼ばれる状況が想像できませんが)。
その共通の基底クラスの名前は CWSHIfObjがいいのですよね。CWSHManager.hというファイル名や以前の CWSH.{h|cpp}の構成から、WSH=WSHMacroとみなされてきたような歴史が想像できますので、おかしいと思う自分が異端なのかもしれませんし、CWSHMacroManagerを抜き出された CWSH.{h|cpp}が今も macroフォルダに入っているのが悪いのかもしれません。
※ CWSHIfObjから抜き出すことになるシンプルな InterfaceObjectですが、CIfObjと CIfObjBaseの二つが名前として挙がっているように思います。rev2パッチでは CIfObjを採用しました。
※「あぁ、WSHに渡すオブジェクトのことね」と受け取ってもらうことを期待して CWSHObjectという名前を提案したのですが、名前には拘りません。
! CEditView*について
今の CWSHIfObj、ひいてはスクリプトに公開される全てのオブジェクトが void*を通して CEditView*が渡されることを前提にしていますが、その担保がどこにもないことを問題視しています。自分の希望とは関係ありませんが、そういう実装を継承させられてしまうので他人事ではありませんでした。
以前は CEditView*を渡す部分と受け取る部分が同じファイルに存在していましたし、受け取る部分(MacroCommand)は staticが付いておりモジュールに固有の関数でしたので、誤用による危険も最小限に抑えられていました。ですから、ファイルやクラスの再構成に伴って生じた問題として、CWSHIfObjのメンバ構成を再度見直す助けにできるかと思って指摘させてもらいました。
この予想される、けれどもまだ起こっていない問題の芽はなくなっていません。「CWSHClient.Execute()の前に CWSHIfObj.ReadyMethods()を呼んでね」という約束が守られなかったときが破滅の始まりです。
! rev2概要
* CWSHIfObjから CIfObjを抜き出して macro/CIfObj.{h|cpp}へ。
* CWSHClientは所有するオブジェクト(複数)の持ち方を自分で決めます。
* CWSHClient.AddInterfaceObject()は受け取ったオブジェクトの AddRef()を呼びます。
* CMethodInfo::IDの型が EFunctionCodeから intになりました。
flagを混ぜ込んだときからもう怪しかったです。最初期も intでした。
* できあがるバイナリサイズが trunk2@r1755と 1バイトたりとも変わりません。空しいです。
* スクリプトが呼ぶメソッドの実体としての(非仮想)メンバ関数が thisポインタを参照するとき、
その値が正しく期待されたものであるかよくわかりません。CWSHIfObjや CIfObjとは異なる
基本クラスから継承した仮想関数を呼べなかったりするかもしれません。
(本当に、呼べるべきなのかどうかも含めてわからない)
!! rev1(無印)との違い
* COutlineIfObj.hのリンクエラーに対処する部分は不要になったので削りました。
* 無駄だった templateも削りました。
* CEditorIfObjが COutlineIfObjなどと同じく CWSHIfObjを継承します。
パッチrev2を確認しました。
旧CInterfaceObjectがCIfObjとして復活し、ソースも分離されてわかりやすくなっていると思います。
CWSHIfObjの名前はとくにこだわりはありません。IfObjだけでWSH関係とわかってもらえるなら CMacroIfObjとかCIfObjExとかでもいいですし、そのままでもいいです。
CEditView*については、CWSHClientのDataをあえて使う必要もないのでメンバ変数になってよかったと思います。Dataは不要なら削除したらと思いましたが、CWSHClientコンストラクタに渡すエラーハンドラの中でやはりDataをCEditView*にキャストしているので削除できないですね。エラーハンドラ中でしか使わないのならvoid*じゃなくてHWNDにしたほうが安全な気もします。
名前は macro/CWSHIfObjのままでいいんじゃないでしょうか。
エラーハンドラの CEditView*についてですが、void* Dataとして CEditView*を渡す部分と、void* Dataを CEditView*として受け取る部分が、CWSHManager.cppの中で完結しているので危険性は最小限です。同一ソースファイル内のコードは、同じクラスの他のメンバ関数などと同じく信頼すべき対象だと思うので、危険があるという認識もほとんどありません。
ここで void*を HWNDという Windows依存な型、特定の利用法に依存した型にしてしまうと、CIfObj::CMethodInfo.IDの型が EFunctionCodeだったときのように、歪んだ(COutlineIfObj::FuncIdを EFunctionCodeに偽装するような)利用法を強制してしまう可能性がでてきます。void*は型安全性を脅かすまずい方法ですが(boost::anyの出番かも)、別の型を装っていないという点で嘘はないので、注意すべき対象だということは伝わります。エラーハンドラに渡す void*は温存しませんか。
対して、スクリプトが呼ぶメソッドの実体(CIfObjMethod型のメンバ関数)に渡される方の void* Dataは削ってもいいものです(削りませんでしたが)。エラーハンドラに渡される Dataと違い出所の怪しい値ですので。怪しいというのは、CIfObj::Invoke()で this->m_Owner->{m_Data|GetData()}とたどる m_Ownerが、実際のスクリプトホスト(CWSHClient)を指していると保証されないことを言っています。であれば、m_Ownerの代わりに m_ObjDataを保持するほうが自由度が高く、紛らわしくもなく直接的です。ここまで書いてきましたが、これに関して深入りするつもりはあまりありません。
syatさん、何も生み出さない議論にお付き合いいただきありがとうございます。反対がなければ数日のうちに rev2をコミットしたいと考えています。そこはまだ解決していないという点があれば、その指摘だけでも早めにお願いします。
他はとくにコメントありません。
rev2パッチでコミットOKと思います。
rev2を revision 1757でコミットしました。