Top > C++ > thisキーワードの危険性

thisキーワードの危険性 Edit

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

コード１
すべてを展開すべてを収束 
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68

 
 
 
 
 
-
|
|
|
|
|
!
 
 
 
-
|
|
|
-
!
|
|
|
-
|
!
|
|
|
!
 
 
 
-
|
|
|
|
-
!
|
|
|
-
|
!
|
|
|
-
|
!
|
|
|
|
!
 
 
-
|
|
|
!
 
 
 

#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
すべてを展開すべてを収束 
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31

 
 
 
-
|
|
|
!
 
 
 
-
|
|
|
|
-
!
|
|
|
-
|
|
!
|
|
|
!
 
 

// コード１の続き
/// メッセージを返すインターフェースクラス。
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キーワードを使うようなメソッドを呼んではいけない？ Edit

それを実践してみたのがコード３になります。

コード３
すべてを展開すべてを収束 
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83

 
 
 
 
 
-
|
|
|
|
|
!
 
 
 
-
|
|
|
-
!
|
|
|
-
|
-
|
!
!
|
|
|
-
|
!
|
|
|
!
 
 
 
-
|
|
|
|
-
!
|
|
|
-
|
!
|
|
|
-
|
!
|
|
|
-
|
!
|
|
|
|
!
 
 
-
|
|
|
|
!
 
 

#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

これでめでたしめでたしかと思いきや、また穴をつくようなコード４を書いてみました。

コード４
すべてを展開すべてを収束 
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31

 
 
 
-
|
|
|
!
 
 
 
-
|
|
|
|
-
|
!
|
|
|
-
|
|
!
|
|
|
!
 

// コード３の続き
/// メッセージを返すインターフェースクラス。
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キーワードをコンストラクタで使っていい場合がある？ Edit

しかし、次のコード５は問題なさそう。

コード５
すべてを展開すべてを収束 
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63

 
 
 
 
 
 
 
 
 
 
-
|
|
-
!
|
|
|
-
|
!
|
|
|
|
|
|
!
 
 
 
-
|
|
-
|
!
|
|
|
-
|
!
|
!
 
 
-
|
!
 
 
 
-
|
|
|
|
!
 
 
 
 

#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
すべてを展開すべてを収束 
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65

 
 
 
 
 
 
-
|
|
|
!
 
 
 
 
-
|
|
-
!
|
|
|
-
|
!
|
|
|
-
|
!
|
|
|
!
 
 
 
-
|
|
-
|
!
|
|
|
-
|
!
|
!
 
 
 
-
|
|
|
|
!
 
 
 

#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関数をオーバーライド禁止にする）

ってことは、コード３も同様に安全になるのでは？
リロード   新規 下位ページ作成 編集 凍結 差分 添付 コピー 名前変更   ホーム 一覧 検索 最終更新 バックアップ リンク元   ヘルプ   最終更新のRSS
Last-modified: Mon, 19 Nov 2007 00:42:45 JST (5996d)