- 追加された行はこの色です。
- 削除された行はこの色です。
* thisキーワードの危険性 [#i9971c47]
#contents
** 問題提起 [#i8249ae9]
** メンバ初期化子およびコンストラクタでthisキーワードを使うのはよくない? [#ub72cb79]
:コード1|
#code(c,){{
#include <iostream>
//
/// ボタンのイベントを受け取るクラス。
class IButtonListener
{
public:
virtual ~IButtonListener(){}
/// ボタンが押されたときに呼ばれる。
virtual void onButtonPushed()=0;
};
/// ボタンクラス。
class Button
{
public:
Button( IButtonListener& aListener )
: listener_( aListener )
{
}
/// 押しボタンイベントを発行する。
void push()
{
listener_.onButtonPushed();
};
private:
IButtonListener& listener_;
};
/// ボタンが押されたらメッセージを出力するクラス。
class MessageButton : public IButtonListener
{
public:
MessageButton( const char* aMessage )
: button_( *this )
, message_( aMessage )
{
}
// IButtonListener
virtual void onButtonPushed()
{
std::cout << message_ << "\n";
}
/// ボタンを押す。
void push()
{
button_.push();
}
private:
Button button_;
const std::string message_;
};
/// メイン関数。
int main (int argc, char * const argv[]) {
MessageButton button( "Button Pushed Message" );
button.push();
return 0;
}
// EOF
}}
''コード1''のMessageButtonクラスに注目。
先にbutton_を初期化してからmessage_を初期化しています。
そのため、もしButtonクラスのコンストラクタの中でIButtonListener::onButtonPushed()が
呼ばれるような実装の変更があった場合、未初期化のmessage_を出力することになってしまいます。
コーディングに厳しい方ならば
1)message_のコンストラクタを先にすればいい
2)Buttonクラスのコンストラクタの仕様が変わったなら他の部分に修正がいくのはしょうがない
という意見も出てくるかと思います。
1)について。
message_を先にコンストラクトしたとしても、
MessageButtonクラスのコンストラクタは完了していない状態です。
その状態で他のオブジェクトからメソッドを呼ばれるのはものすごく嫌な感じです。
嫌な感じがする中、穴をついたようなコードが''コード2''になります。
:コード2|
#code(c,){{
// コード1の続き
/// メッセージを返すインターフェースクラス。
class IMessage
{
public:
virtual ~IMessage(){}
virtual std::string createMessage()const=0;
};
/// HelloWorldを出力し、その後任意のメッセージを出力するMessageButton。
class HelloWorldButton : public MessageButton
{
public:
HelloWorldButton( const IMessage& aIMesssage )
: MessageButton( "Hello World" )
, message_( aIMesssage )
{
}
// MessageButton
virtual void onButtonPushed()
{
MessageButton::onButtonPushed();
std::cout << message_.createMessage();
}
private:
const IMessage& message_;
};
}}
仮想関数をさらにオーバーライドしてみました。
Buttonのコンストラクタでリスナのメソッドを呼ばれようものなら
コード2のメンバ初期化が始まる以前にonButtonPushedが呼ばれてしまいます。
これらのことをふまえて、
thisなものをメンバ変数に渡すときは、その時点でリスナに対する呼び出しの準備が完了している必要がある(リスナは初期化済みの状態であるべき) ...(*1)
と、私は考えました。
(*1)を確実に実現するためには
メンバ初期化子・コンストラクタでthisキーワードを使わない。 ...(*2)
言い換えると
thisキーワードはコンストラクトが完了してからのみ使える。 ...(*3)
となります。
コンストラクタでthis使わなければハッピーじゃん、という考え方。実にシンプル。
ちなみに、メンバ初期化子でthis使うとVC8.0では警告を出してくれます。すばらしい。
** コンストラクタの中でthisキーワードを使うようなメソッドを呼んではいけない? [#peba7bcc]
それを実践してみたのが''コード3''になります。
:コード3|
#code(c,){{
#include <iostream>
//
/// ボタンのイベントを受け取るクラス。
class IButtonListener
{
public:
virtual ~IButtonListener(){}
/// ボタンが押されたときに呼ばれる。
virtual void onButtonPushed()=0;
};
/// ボタンクラス。
class Button
{
public:
Button()
: listener_(0)
{
}
/// 押しボタンイベントを発行する。
void push()
{
if ( listener_ != 0 )
{
listener_->onButtonPushed();
}
};
/// リスナを登録する。
void registerListener( IButtonListener& aListener )
{
listener_ = &aListener;
}
private:
IButtonListener* listener_;
};
/// ボタンが押されたらメッセージを出力するクラス。
class MessageButton : public IButtonListener
{
public:
MessageButton( const char* aMessage )
: button_()
, message_( aMessage )
{
}
// IButtonListener
virtual void onButtonPushed()
{
std::cout << message_ << "\n";
}
/// ボタンを押す。
void push()
{
button_.push();
}
/// リスナを登録する。
void registerListener()
{
button_.registerListener(*this);
}
private:
Button button_;
const std::string message_;
};
/// メイン関数。
int main (int argc, char * const argv[]) {
MessageButton button( "Button Pushed Message" );
button.registerListener();
button.push();
return 0;
}
// EOF
}}
これでめでたしめでたしかと思いきや、また穴をつくような''コード4''を書いてみました。
:コード4|
#code(c,){{
// コード3の続き
/// メッセージを返すインターフェースクラス。
class IMessage
{
public:
virtual ~IMessage(){}
virtual std::string createMessage()const=0;
};
/// HelloWorldを出力し、その後任意のメッセージを出力するMessageButton。
class HelloWorldButton : public MessageButton
{
public:
HelloWorldButton( const IMessage& aIMesssage )
: MessageButton( "Hello World" )
, message_( aIMesssage )
{
registerListener(); // やってること変わらんやん!
}
// MessageButton
virtual void onButtonPushed()
{
MessageButton::onButtonPushed();
std::cout << message_.createMessage();
}
private:
const IMessage& message_;
};
}}
コード4のHelloWorldButtonを継承し
onButtonPushedの実装を書いたらまた同じことになりえます。
これを防ぐには
コンストラクタの関数でthisを使う関数を呼ばない ...(4)
がルールとして加わることになります。
もっとシンプルに行くと
コンストラクタの関数で非constな関数を呼ばない ...(5)
となりますね。かなりの制約になりますが・・・。
** thisキーワードをコンストラクタで使っていい場合がある? [#k47b70c8]
しかし、次の''コード5''は問題なさそう。
:コード5|
#code(c,){{
#include <iostream>
#include <list>
#include <algorithm>
//
class Button;
/// ボタンの管理クラス。
class ButtonManager
{
public:
ButtonManager()
{
};
/// ボタンを追加する。
void add( Button& aButton )
{
buttons_.push_back( &aButton );
}
/// 全ボタンを押す。
void pushAll();
private:
std::list< Button* > buttons_;
};
/// ボタンクラス。
class Button
{
public:
Button( ButtonManager& aManager )
{
aManager.add( *this );
}
/// 押しボタンイベントを発行する。
void push()
{
std::cout << "Pushed\n";
};
};
void ButtonManager::pushAll()
{
std::for_each( buttons_.begin() , buttons_.end() , std::mem_fun( &Button::push ) );
}
/// メイン関数。
int main (int argc, char * const argv[]) {
ButtonManager manager;
Button button(manager);
manager.pushAll();
return 0;
}
// EOF
}}
このケースが問題でないのは、thisを登録する先が呼ぶメソッドが非仮想関数だからです。
そうか、ってことは''コード6''はよろしくないんだ。
:コード6|
#code(c,){{
#include <iostream>
#include <list>
#include <algorithm>
//
class IButton
{
public:
virtual ~IButton(){}
virtual void buttonPush()=0;
};
/// ボタンの管理クラス。
class ButtonManager
{
public:
ButtonManager()
{
};
/// ボタンを追加する。
void add( IButton& aButton )
{
buttons_.push_back( &aButton );
}
/// 全ボタンを押す。
void pushAll()
{
std::for_each( buttons_.begin() , buttons_.end() , std::mem_fun( &IButton::buttonPush ) );
}
private:
std::list< IButton* > buttons_;
};
/// ボタンクラス。
class Button : public IButton
{
public:
Button( ButtonManager& aManager )
{
aManager.add( *this );
}
/// 押しボタンイベントを発行する。
void buttonPush()
{
std::cout << "Pushed\n";
};
};
/// メイン関数。
int main (int argc, char * const argv[]) {
ButtonManager manager;
Button button(manager);
manager.pushAll();
return 0;
}
// EOF
}}
Buttonクラスを継承してbuttonPush関数をオーバーライドするようなパターンが危険ですね。
ということは、VC8.0のfinalキーワードのように
仮想関数をオーバーライドできないようにする機能をコンパイラがサポートしていれば
コード6も大丈夫になりますね。(Button::buttonPush関数をオーバーライド禁止にする)
ってことは、コード3も同様に安全になるのでは?