スレッド: [cppll:12985] std::map のキーにオブジェクトのインスタンスを使用する

こんにちは。中山です。

std::mapの使用方法に関してご教示頂きたくメールする次第です。
下記のように

map< CLevelPos, InfoPtr > InfoTable;

「CLevelPos」クラスのオブジェクトを検索キーにして「InfoPtr」を保持・取得
できる変数「InfoTable」を宣言しました。

CLevelPosは下記のように2つの「short型」変数を持っています。
//ザックリと全メンバを「public」にしてますが実際は異なります。
class CLevelPos{
public:
   CLevelPos( short level_, short pos_ ) : level(level_), pos(pos_) {}
   unsigned short level;
   unsigned short pos;
};


そこで、下記の関数の中身の書き方に疑問を持ちまして質問させていただく次第
です。
bool CLevelPos::operator < (const CLevelPos &src ) const ;


当初、下記のように書いてみましたが思った通りに動作しませんでした。
具体的には、「InfoTable.find(oKey);」時に、全く見当はずれのKeyが「合致し
た」という結果を返してきました。

bool CLevelPos::operator < (const CLevelPos &src ) const {
   if( level < src.level ) return true;
   if( pos < src.pos ) return true;
   return false;
}


下記のように書き換えたところ、問題なく動作するようになりましたが、
検索キー用のオブジェクトである「CLevelPos」のメンバ変数が増えた場合に下
記の方法では限界がくるので、どのようにすべきかアドバイス頂きたくお願いい
たします。

bool CLevelPos::operator < (const CLevelPos &src ) const {
   // 二つの値を一つの値に変換して大小を比較する
   unsigned long d = ((level & 0xffff) << 16) | (pos & 0xffff);
   unsigned long s = ((src.level & 0xffff) << 16) | (src.pos & 0xffff);

   return d < s;
}


以上、長文失礼致しました。
アドバイスをいただければ有難く思います。
宜しくお願いいたします。


--
take_de_x <take_de_x@...>

とっちゃんです。

こちらでは、はじめましてですね。みなさんよろしく〜ｗ

>
>当初、下記のように書いてみましたが思った通りに動作しませんでした。
>具体的には、「InfoTable.find(oKey);」時に、全く見当はずれのKeyが「合致し
>た」という結果を返してきました。
>
>bool CLevelPos::operator < (const CLevelPos &src ) const {
>    if( level < src.level ) return true;
>    if( pos < src.pos ) return true;
>    return false;
>}
>
mapや、set でキーとなるためというより、

obj1 < obj2 が成立する場合、
obj2 < obj1 が絶対に成立しないようにしなければなりませんが、
現在の内部比較条件だと成立する場合があります。なので、結果が不安定になってる。

if( level < src.level ) return true;
else if( level > src.level ) return false;

としてやれば、安定しますよ。
理由は分かりますよね？


// とっちゃん(高萩 俊行)
// http://blogs.wankuma.com/tocchann/
// Microsoft MVP for Windows-SDK (Oct 2005 - Sept 2007)
// WindowsInstallerの話題は http://www.freeml.com/ctrl/html/MLInfoForm/msi@freeml.com まで

こんにちは。中山です。

皆さん、素早いレスポンスありがとうございます。

> obj1 < obj2 が成立する場合、
> obj2 < obj1 が絶対に成立しないようにしなければなりませんが、

仰るとおりです。
そのように operator < () を定義したかったのですが、自分の視野が狭かった
ようです^^;

map,set が検索を行う時は
・「X1 < X2」「X2 < X1」の両方が成立しなかったとき「X1==X2」とみなす
ってのが標準的な動作だと一応は認識はしていて(合ってますか？)、先のメール
のようなoperatorを定義したのですが、考えが足らなかったです。

メンバ変数が増えたときのことを考慮して下記のように書いておくのが良いでしょ
うか。

if( level < src.level ) return true;
else if( level > src.level ) return false;
return pos < src.pos;


例えば「value」が増えたら下記のように書き換えるとか…


if( level < src.level ) return true;
else if( level > src.level ) return false;
if( pos < src.pos ) return true;
else if( pos > src.pos ) return false;
return value < src.value;

皆さん、ありがとうございました。

--
take_de_x <take_de_x@...>

札幌の松浦です。

＃ 最近暑いっす、ほんとに札幌かよ？

> map,set が検索を行う時は
> ・「X1 < X2」「X2 < X1」の両方が成立しなかったとき「X1==X2」とみなす
> ってのが標準的な動作だと一応は認識はしていて(合ってますか？)、

はい、あっています。標準的というか標準です。

このあたりは、「プログラミング言語C++第3版」 17.1.4.1, 17.1.4.2
に詳しく書いてあります。


--
 松浦 明彦 <matuura@...>

こんにちは。中山です。

> > map,set が検索を行う時は
> > ・「X1 < X2」「X2 < X1」の両方が成立しなかったとき「X1==X2」とみなす
> > ってのが標準的な動作だと一応は認識はしていて(合ってますか？)、
> はい、あっています。標準的というか標準です。
> このあたりは、「プログラミング言語C++第3版」 17.1.4.1, 17.1.4.2
> に詳しく書いてあります。

なるほど、ありがとうございます。
再勉強したいと思います。


--
take_de_x <take_de_x@...>

稲田です。

蛇足ですが、else は意味が無いので、つけないほうが見た目がすっきりしますね。

if( level < src.level ) return true;
if( level > src.level ) return false;

さらに、operator < と operator > の両方を使う意味が無いので、 operator < に統一すると、
επιστημη さんの例の書き方になります。

ちょっとしたことですが、これができていると「センスが良い」と思います。

--
INADA Naoki <inadauer@...>
Blog: http://d.hatena.ne.jp/methane/ (sorry, Japanese only.)

こんにちは、中山です。

稲田さん、とっちゃんさん、ソースコードの記述方法に関してのアドバイスあり
がとうございました。
非常に勉強になりました。

見やすく、間違いが発生しにくいコードを書くように気をつけたいと思います。

--
take_de_x <take_de_x@...>

とっちゃんです。

>
>if( level < src.level ) return true;
>else if( level > src.level ) return false;
>return pos < src.pos;
>
>
>例えば「value」が増えたら下記のように書き換えるとか…
>
>
>if( level < src.level ) return true;
>else if( level > src.level ) return false;
>if( pos < src.pos ) return true;
>else if( pos > src.pos ) return false;
>return value < src.value;
>

書き方として、else や operator<() で統一するかは、
コーディングスタイルなどもあるので、どちらでもいいとは思いますが、

構文そのものは全部統一しておくことをお勧めします。

上記の例でいえば、

if( level < src.level ) return true;
else if( level > src.level ) return false;

if( pos < src.pos ) return true;
else if( pos > src.pos ) return false;

if( value < src.value ) return true;
else if( value > src.value ) return false;

return false;

という感じですね。

επιστημηさん風にかくなら、

if( level < src.level ) return true;
if( src.level < level ) return false;

if( pos < src.pos ) return true;
if( src.pos < pos ) return false;

...

return false;

という感じ。

若干冗長な見た目になりますが、いまどきのコンパイラなら
この程度なら余裕で最適化してくれます。

近い将来のレベルで変更が予定されている(要するに作りかけのクラス)のなら
こういう形にしておくことで、単純にコピーして増やしていけますから
結構便利です。

また、構文を合わせることで、ソース上に同じパターンが出現するので
ミスを抑えやすくするという利点も生まれます。


// とっちゃん(高萩 俊行)
// http://blogs.wankuma.com/tocchann/
// Microsoft MVP for Windows-SDK (Oct 2005 - Sept 2007)
// WindowsInstallerの話題は http://www.freeml.com/ctrl/html/MLInfoForm/msi@freeml.com まで

επιστημηです。

> 構文そのものは全部統一しておくことをお勧めします。
>...
> if( level < src.level ) return true;
> if( src.level < level ) return false;
>
> if( pos < src.pos ) return true;
> if( src.pos < pos ) return false;
> ...
> return false;
>
> という感じ。
>
> 若干冗長な見た目になりますが、いまどきのコンパイラなら
> この程度なら余裕で最適化してくれます。

ですねー。
今後(近い将来)の拡張が見込まれてるならきっちり形式化しておくが吉でしょね。
メンバ追加時にどういぢればいいかがクリアだから。

cbと申します。お久しぶりです。

横から質問ですみません。


私の場合、比較演算子のオーバロードはメンバ関数ではなくフレンド関数にします。

bool CLevelPos::operator < (const CLevelPos &src ) const {
       if ( level < src.level ) return true;
       if ( src.level < level ) return false;
       return pos < src.pos;
}

CLevelPos {
       /* 略 */
       friend bool operator < ( const CLevelPos& srcL, const CLevelPos& srcR );
}
bool operator < ( const CLevelPos& srcL, const CLevelPos& srcR ) {
       if ( srcL.level < srcR.level ) return true;
       if ( srcR.level < srcL.level ) return false;
       return srcL.pos < srcR.pos;
}


メンバ関数とフレンド関数でのそれぞれのメリット、デメリットなどありますでしょうか？

> メンバ関数とフレンド関数でのそれぞれのメリット、デメリットなどありますで
しょうか？

演算子の両辺が異なる型のとき、メンバ関数では対応できなくなることがあります。

class String {
public:
 bool operator==(const char* rhs);
 ...
};

なんて定義だと x == "ほげ" できるけど "ほげ" == x できんです。

cbです。


> 演算子の両辺が異なる型のとき、メンバ関数では対応できなくなることがあります。
>
> class String {
> public:
>   bool operator==(const char* rhs);
>   ...
> };
>
> なんて定義だと x == "ほげ" できるけど "ほげ" == x できんです。
>
ああ、ありましたね。

bool operator == ( const String& srcL, const char& srcR ) { }
bool operator == ( const char* srcL, const String& srcR ) { }
としておけば、両方に対応できるってやつですね。

笠原、です。

On 6/14/07, take_de_x <take_de_x@...> wrote:
> メンバ変数が増えたときのことを考慮して下記のように書いておくのが良いでしょ
> うか。
>
> if( level < src.level ) return true;
> else if( level > src.level ) return false;
> return pos < src.pos;

少しわき道に逸れますが、メンバー変数 A, B, C ...., Z があるときには
operator == が定義されていれば

if( A != src.A ) return A < src.A;
if( B != src.B ) return B < src.B;
・
・
if( Z != src.Z ) return Z < src.Z;
return false;

という書き方がコンパクトで、変数の追加削除に強いのでお勧めです。
operator < しかない場合には επιστημη さん方式でないと無理ですが。

--
KASAHARA Masahiro <mkasa@...>

　こんにちは、電柱一家です。

> bool CLevelPos::operator < (const CLevelPos &src ) const {
>     if( level < src.level ) return true;
>     if( pos < src.pos ) return true;
>     return false;
> }

　これだと、level > src.level のとき、 pos < src.pos で true
　を返すからまずいのかな？
　if ( level < src.level )
　　return true;
　if ( level == src.level && pos < src.pos )
　　return true;
　とか、かな？

επιστημηです。

> 当初、下記のように書いてみましたが思った通りに動作しませんでした。
> 具体的には、「InfoTable.find(oKey);」時に、全く見当はずれのKeyが「合致し
> た」という結果を返してきました。
>
> bool CLevelPos::operator < (const CLevelPos &src ) const {
>     if( level < src.level ) return true;
>     if( pos < src.pos ) return true;
>     return false;
> }

えと、まずlevelの大小で判定し、決着がつかない(levelが等値)ならposで判断す
りゃいいのですよね。

if ( level < src.level ) return true;
if ( src.level < level ) return false;
return pos < src.pos;

これでいいんちゃう? メンバ変数増えてもこの要領で。

初投稿させていただきます。Akifuと申します。

> bool CLevelPos::operator < (const CLevelPos &src ) const {
>     if( level < src.level ) return true;
>     if( pos < src.pos ) return true;
>     return false;
> }
これでは、level > src.levelのとき（明らかにfalseを返すべきときだと思われる）に、
posの大小を見ることになってしまいます。
正しくは、

bool CLevelPos::operator < (const CLevelPos &src ) const {
    if( level < src.level ) return true;

    if( level == src.level ) {
       return pos < src.pos;
    }
    return false;
}
としたいのではないでしょうか？


--
###################
  akifu@...
  Watanabe Akifumi
###################