C++/thisキーワードの危険性の変更点

Top > C++ > thisキーワードの危険性
追加された行はこの色です。
削除された行はこの色です。
C++/thisキーワードの危険性へ行く。
C++/thisキーワードの危険性の差分を削除
* thisキーワードの危険性 [#i9971c47]
#contents

** メンバ初期化子およびコンストラクタでthisキーワードを使うのはよくない？ [#ub72cb79]

:コード１|
#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

}}

''コード１''のMessageButtonクラスに注目。
先にbutton_を初期化してからmessage_を初期化しています。
そのため、もしButtonクラスのコンストラクタの中でIButtonListener::onButtonPushed()が
呼ばれるような実装の変更があった場合、未初期化のmessage_を出力することになってしまいます。

コーディングに厳しい方ならば
１）message_のコンストラクタを先にすればいい
２）Buttonクラスのコンストラクタの仕様が変わったなら他の部分に修正がいくのはしょうがない
という意見も出てくるかと思います。

１）について。
message_を先にコンストラクトしたとしても、
MessageButtonクラスのコンストラクタは完了していない状態です。
その状態で他のオブジェクトからメソッドを呼ばれるのはものすごく嫌な感じです。

嫌な感じがする中、穴をついたようなコードが''コード２''になります。

:コード2|
#code(c,){{
// コード１の続き
/// メッセージを返すインターフェースクラス。
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のコンストラクタでリスナのメソッドを呼ばれようものなら
コード２のメンバ初期化が始まる以前にonButtonPushedが呼ばれてしまいます。

これらのことをふまえて、
 thisなものをメンバ変数に渡すときは、その時点でリスナに対する呼び出しの準備が完了している必要がある（リスナは初期化済みの状態であるべき） ...(*1)
と、私は考えました。

(*1)を確実に実現するためには
 メンバ初期化子・コンストラクタでthisキーワードを使わない。 ...(*2)
言い換えると
 thisキーワードはコンストラクトが完了してからのみ使える。 ...(*3)
となります。

コンストラクタでthis使わなければハッピーじゃん、という考え方。実にシンプル。
ちなみに、メンバ初期化子でthis使うとVC8.0では警告を出してくれます。すばらしい。

** コンストラクタの中でthisキーワードを使うようなメソッドを呼んではいけない？ [#peba7bcc]
それを実践してみたのが''コード３''になります。
:コード３|
#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
}}

これでめでたしめでたしかと思いきや、また穴をつくような''コード４''を書いてみました。
:コード４|
#code(c,){{
// コード３の続き
/// メッセージを返すインターフェースクラス。
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_;
};
 
}}

コード４のHelloWorldButtonを継承し
onButtonPushedの実装を書いたらまた同じことになりえます。

これを防ぐには
 コンストラクタの関数でthisを使う関数を呼ばない ...(4)
がルールとして加わることになります。
もっとシンプルに行くと
 コンストラクタの関数で非constな関数を呼ばない ...(5)
となりますね。かなりの制約になりますが･･･。

** thisキーワードをコンストラクタで使っていい場合 [#k47b70c8]
** thisキーワードをコンストラクタで使っていい場合がある？ [#k47b70c8]
しかし、次の''コード５''は問題なさそう。
:コード５|
#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|
#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キーワードのように
仮想関数をオーバーライドできないようにする機能をコンパイラがサポートしていれば
コード６も大丈夫になりますね。（Button::buttonPush関数をオーバーライド禁止にする）

ってことは、コード３も同様に安全になるのでは？
C++/thisキーワードの危険性 の変更点

C++/thisキーワードの危険性の変更点
