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

thisキーワードの危険性 Edit

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

コード1
Everything is expanded.Everything is shortened.
  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

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

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

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

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

コード2
Everything is expanded.Everything is shortened.
  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
 
 
 
-
|
|
|
!
 
 
 
-
|
|
|
|
-
!
|
|
|
-
|
|
!
|
|
|
!
 
 
// コード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キーワードを使うようなメソッドを呼んではいけない? Edit

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

コード3
Everything is expanded.Everything is shortened.
  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

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

コード4
Everything is expanded.Everything is shortened.
  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
 
 
 
-
|
|
|
!
 
 
 
-
|
|
|
|
-
|
!
|
|
|
-
|
|
!
|
|
|
!
 
// コード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キーワードをコンストラクタで使っていい場合がある? Edit

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

コード5
Everything is expanded.Everything is shortened.
  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はよろしくないんだ。

コード6
Everything is expanded.Everything is shortened.
  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キーワードのように
仮想関数をオーバーライドできないようにする機能をコンパイラがサポートしていれば
コード6も大丈夫になりますね。(Button::buttonPush関数をオーバーライド禁止にする)

ってことは、コード3も同様に安全になるのでは?


Reload   New Lower page making Edit Freeze Diff Upload Copy Rename   Front page List of pages Search Recent changes Backup Referer   Help   RSS of recent changes
Last-modified: (5388d)