-1

オーバーロードされた論理比較演算子 (==) のコードを次に示します。これを使用して、2 つの文字列のサイズと内容が同一かどうかを確認します。それ以外の場合は false を返す必要があります。

 bool MyString::operator==(const MyString& other)const
 {
    if(other.Size == this->Size)
    {
        for(int i = 0; i < this->Size+1; i++)
        {
            if(&other == this)                          
                  return true;            
        }
    }
    else
        return false;
 }

valgrind を実行すると、警告コントロールが非 void 関数の終わりに達したことが通知されました。この問題を修正する方法と、コードを改善するために何ができるかについての提案はありますか?

4

4 に答える 4

3

制御がforループの最後に到達すると、値を返さずにすぐに関数の最後に到達します。

forとにかく、ループ内のロジックが変更されているように見えます-他のアイテムのアドレスをこれと比較しています。それを行っても問題ありませんが、ループではなく、一度だけ行う必要があります。

ループでは、オブジェクトのアドレスではなく、文字列内の文字を比較したいことは間違いありません。

編集：

典型的な実装は、次の一般的な順序で行われます。

class MyString { 
    char *data;
    size_t length;
public:
    // ...
    bool operator==(MyString const &other) const {
        if (length != other.length)
            return false;
        for (int i=0; i<length; i++)
            if (data[i] != other.data[i]) // If we see any inequality
                return false;             //     they're not equal
        return true;                      // all equal, so the strings are equal.
    }
};
于 2012-07-17T14:31:43.843 に答える
1

サイズが等しい場合に何が等しいかはあまり明確ではありませんが、ループは次のようなものを探していることを示唆しています。

bool
MyString::operator==( MyString const& other ) const
{
    return size == other.size && std::equals( ??? );
}
于 2012-07-17T14:36:28.337 に答える
1

まず、forループに入って条件&other == thisが満たされない場合、何も返されません。これを修正するには、elseステートメントを削除するだけです。other.Size == this->Sizeこれにより、条件が満たされない場合、またはループ全体を通過し、ループ内で使用されていない場合、関数は false を返します return。

2 番目の問題は、行if(&other == this)です。ループ内で、文字列のすべてのシンボルをチェックするつもりだと思います。しかし今は、クラス自体へのポインターのみをチェックしています。文字をチェックするには、データを保存if( other->data == this->data )するメンバーがある場合、 のようなものを使用する必要がありdataます (トートロジーで申し訳ありません)。

もう 1 つの小さな流れがデザインにあります。文字列が等しいことを確認するには、すべての文字を調べて一致することを確認する必要があります。ただし、文字列が等しくないことを証明するには、一致しない文字のペアを 1 つだけ見つける必要があります。その後、比較を続けても意味がありません。したがって、一致しないペアを見つけたらすぐに比較を停止し、他の文字の無駄な比較を行わないように、サイクル内の条件を負の条件に変更することをお勧めします。

一般に、すべてのエラーをできるだけ早く返し、不要な計算を避けることをお勧めします。したがって、単純なチェックで関数の最初に何かをチェックできる場合は、それを実行することをお勧めします。

したがって、結局のところ、次のようなものが必要です。

bool MyString::operator==(const MyString& other)const
{
   if(other.Size != this->Size)
       return false;//If the sizes do not match, no need to check anything else. Just return false.

   //If we are here, the sizes match. Lets check the characters.
   for(int i = 0; i < this->Size+1; i++)
   {
       //If some pair doesnt match, the strings are not equal, and we exit.
       if( other->data[i] != this->data[i])                          
                  return false;
   }

   //If we are here, all the characters did match, so we return true.
   return true;
}
于 2012-07-17T15:47:17.870 に答える
0

を取り除くだけですelse。falseこのようにして、条件が満たされない場合に返される「デフォルト」の動作があります。それはあなたが意図した機能であり、コンパイラや構文チェッカーは文句を言いません。

于 2012-07-17T14:31:50.373 に答える