bSelLock
はたしかに else や default でパターンを網羅したいケース。■SetFocus
は、他はなくても必ず自分自身が対象として含まれるだろうという期待があるのだろうけど、それは予断だし油断があったのだと思う。同じファイルのすぐ上にある CViewCommander::Command_CASCADE
にも同じ型のコードがあるけどそれは?■クリア済みのWndProcを呼ぶことについて。呼べないから呼ばないでいいのか、呼べないから呼べるようにして呼ぶのか、判断がつかない。■パッチ部分は SendMessageToAllEditors
なのだけどそのすぐ上にある Send を Post に置き換えただけの PostMessageToAllEditors
については?■ここまでで触れずにとばした部分は全体に趣味的な印象を持った。delete 0
は安全らしいし、CloseHandle(0)
は無害らしいし、使わない領域、範囲外とインデックスでマークされてる領域を NULL で埋めなければいけない理由はないし、変数を2つ定義して、同時に初期化した片方を条件にもう一方を直後の if-else 文で初期化(※厳密には代入)するというとき、定義部分でも初期化が必須かといえばどーでもいい。それよりも C99 より前の C言語に倣った関数冒頭にまとめた変数宣言(&定義)のほうがつらい。長大な関数のあるブロックである変数が必要になったとき、使いたい値が代入されてからそこまでのすべてのコードパスでどんな値が代入され得るのかチェックしないといけないし、反対に不要になったときは、代入をやめたことで関数の以後で問題が起こらないことを確認しなければいけないし、それを forループの変数 i についても行わなければいけないのだから。行き当たりばったりで次々変数を増やすのもどうかと思うが、適切な粒度のスコープがあると思う。env/CAppNodeManager.cppのパッチで指摘された初期化はまとまっていて問題を見出せない。■env/CAppNodeManager.cppの別の部分については、IsSakuraMainWindow
でもやっていて実質的な意味を持たない NULLチェックを足して今以上にごたごたさせるより、2番目のループを取り除くぐらいの書き直しをしたらいい。「hWndLastが NULLのときに全くメッセージが送られていなかった」なんてときのコードはおそらく原型が残っていないし(そんなことを言うやつがよくわからないまま(同じ)轍を踏むのだ(自虐)。でも unicodeブランチでは遡れるログに限界が……)、その対応のせいで(たぶん昔はそうだったのだろうけど)双子のループではなくなっているのだから。こんな感じにしたい>CAppNodeGroupHandle__PostMessageToAllEditors.txt。もちろん SendMessageの方も。仕様の違いは、pWndArrに hWndLastが複数含まれていたときに複数回メッセージが送られないってことだけど、そんなこと起こらないし期待もしていないでしょう? ほぼコピペの双子関数をひとつにまとめるのは、PostMessageと SendMessageの戻り値の型が違うせいで失敗した。マクロを使うとデバッガで追いかけるのが難しくなるし、メンバ関数テンプレートはコンパイラの実装が遅かったような記憶があって積極的に使いたくないし、そもそもリンクエラーを取り除けなかった(<テンプレートの定義を.cppに書いていたから。明示的な実体化が必要だった。こんな感じ)。たぶんファイルローカルのモジュール関数テンプレートにしたら一挙両得だけど、アクセス許可を出さないとダメだよね? 代わりに staticメンバにする? そうするだけの、過剰なテクニックを駆使したりヘッダに手を入れたりするだけのメリットがないのでコピペのままでいいや。■macro/CWSH.cppについて。中断のためのスレッド関数とメインスレッドがパラメーターを通して hEventを共有していて、子スレッドの方で
OpenEvent
や DuplicateHandle
みたいなものは呼んでいないんだけど、子スレッドの終了を待たずに SetEvent(hEvent);
即 CloseHandle(hEvent);
して子スレッドはイベントを受け取れる?■読んだのはここまで。趣味的(あるいは潔癖・偏執的)とは書いたけどデバッグモードのコードだと思えばそういうものかなという気がしないでもない。が、C++が初期に遅いと言われたのってコンパイラが隠れてする仕事(コンストラクタの呼び出しとか)に対する無理解が理由にあると聞くので、無駄とわかってるところに 0 を埋めて歩くことに積極的にはなれない。staticでない未初期化変数の値をそのまま使用するのは結果が予測できないバグだけど、未初期化変数(遅延初期化変数)それ自体は問題にしないよ。■自分の不見識が明らかになるだけかもしれないけど……。「スタックのドカ食い対策」<スタックで間に合ってるならスタックの方が速いんじゃないの? 「
WINVERによる条件コンパイル」<ソース分裂バイナリ分裂はない方がいいし、俺プリプロセッサ嫌いなんだよね(<誰も聞いていない! 共感性・説得力皆無!)。さらに共感を得られそうにないことを書くと、namespaceが導入されたからグローバルな名前空間を明示するために
::
を付けましょう、これまでただ Func();
と呼んでいたものを ::Func();
と書きましょう、なんてのはなんの冗談かと思う。譲るのは当然後から来たほうだし、衝突しないような名前を選ぶ、衝突が避けられないときにしぶしぶ ::
を付けて区別する、くらいの態度でいるんだけど、みなさん積極的に ::
を付けますね。それって(知らないしやったことないけど)まさに MFCプログラミングで名前のバッティングが避けられなかったからみんなそうしてただけなんじゃないかとも思ってるんだけど、SDKプログラミングでもわりと積極的だよね。自分が無知で異端なのかと不安になる。■途切れていた古いログを見つけた。「Fix: Send/PostMessageToAllEditors() don't send message when last window parameter is NULL.」この修正が不可解で、現在のソースに残ったコメントから以前のコードを想像できなかった(だから原型が残っていないのだろうとコメントした)。単純に m_hWndLast の NULLチェックを省いてこう直せば良かったじゃない?>if( phWndArr[i] != m_hWndLast )
この頃はまだ IsEditWnd
(※後のIsSakuraMainWindow
?)が引数の NULLチェックをしていないことから、先取りして phWndArr[i](※m_hWndLastではない)の NULLチェックを追加してもいい>if( phWndArr[i] != m_hWndLast && phWndArr[i] )
。ま、済んだことはさておき、この時点ではまだ双子のループが存在していないことが興味深い。双子のループが誕生したのはこのコミット>「New: Grouping of the tab windows.」(←Firefoxだとアドレスバーで20回ぐらい Enterキーを押しているうちにページ内アンカー(#diff-24)が機能するけど、Operaだとリロードしてしまって永遠に機能しない。くそったれな遅延ロード。くそったれな S……)。グルーピング対応のためとはいえループは必要だったろうか(反語ではありません。結論保留)。