2017-05-27 7 views
0

チェスゲームを作っていて、ベクトルに有効な位置を追加しようとしています。私が取り組んでいる特定の作品は騎士であり、ハードコーディングしたポジションの一部は、ボード上のナイトの位置に応じて、ボードから外されています。私の座標系は行にはA-H、列には0〜8を使用します。C++ find_first_not_of(char *)を使ってベクトル要素を取り除く

有効な文字(A〜Hおよび0〜8)の文字配列を定義しており、無効な座標ペアを識別して削除するためにfind_first_not_ofを使用しています。 ?A1

無効:

有効な例:3 - この

質問削除:なぜそれは私の機能が無効であるとパターンに適合しないいくつかの座標のペアを削除するということですが、しかし、他にはない?たとえば、位置A2を入力として使用すると、@ 4と@ 0は正常に削除されます。しかし、残りの利用可能な位置は、C3、C1、B4、θ3、B0、θ1である。

入力がH2の場合、F3、F1、G0、G4は使用可能な位置であり、J3、J1、I4、I0は正常に削除され、有効な位置のみの結果が得られます。

問題:無効な位置を削除するのと同じ方法をとると、私の出力は正しいことがあり、それ以外は正しくありません。

親クラスPiece.cpp:

char Piece::valids[16] = { 
    'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 
    '0', '1', '2', '3', '4', '5', '6', '7' 
}; 

void Piece::removeInvalids(vector<string>& v) 
{ 
    for (short i = 0; i < v.size(); i++) 
    { 
     string s = v.at(i); 
     size_t found = s.find_first_not_of(valids); 
     if (found != string::npos) 
     { 
      cout << v.at(i) << endl; 
      swap(v.at(i), v.back()); 
      v.pop_back(); 
     } 
    } 
} 

子クラスKnight.h:

vector<string> getAvailPositions(Piece **all) 
{ 
    vector<string> v; 
    stringstream ss; 
    ss << static_cast<char>(position[0] + 2) 
      << static_cast<char>(position[1] + 1); 
    v.push_back(ss.str()); 
    stringstream ss2; 
    ss2 << static_cast<char>(position[0] + 2) 
      << static_cast<char>(position[1] - 1); 
    v.push_back(ss2.str()); 
    stringstream ss3; 
    ss3 << static_cast<char>(position[0] + 1) 
      << static_cast<char>(position[1] + 2); 
    v.push_back(ss3.str()); 
    stringstream ss4; 
    ss4 << static_cast<char>(position[0] - 1) 
      << static_cast<char>(position[1] + 2); 
    v.push_back(ss4.str()); 
    stringstream ss5; 
    ss5 << static_cast<char>(position[0] + 1) 
      << static_cast<char>(position[1] - 2); 
    v.push_back(ss5.str()); 
    stringstream ss6; 
    ss6 << static_cast<char>(position[0] - 1) 
      << static_cast<char>(position[1] - 2); 
    v.push_back(ss6.str()); 
    stringstream ss7; 
    ss7 << static_cast<char>(position[0] - 2) 
      << static_cast<char>(position[1] - 1); 
    v.push_back(ss7.str()); 
    stringstream ss8; 
    ss8 << static_cast<char>(position[0] - 2) 
      << static_cast<char>(position[1] + 1); 
    v.push_back(ss8.str()); 
    removeInvalids(v); 
    return v; 
} 

あなたはより良い私を支援するため、この記事への変更がなされるべきである場合は私に知らせてください、 ありがとうございました。

+1

文字の代わりに位置(たとえば、1〜8行と1〜8の列)に整数座標を使用することをお勧めします。そうすれば、ボード外の位置を簡単に計算し、より簡単にモーションをコード化することができます。位置を出力し、ユーザに移動するために、それらをA-Hと1-8に文字列として翻訳することができます。 – Gerriet

+0

'position'配列と' valids'文字列のようなものを含めることができますか?編集:私は 'valids'がそこにあることを確認します。 –

+0

文字列の位置。 Piece.hにあります。その変数の値は常にA0〜H7になります。 OPへの編集でvalids配列を追加しました。 –

答えて

0

デザインは再考する必要があります。複雑で非効率です。これはどう:

typedef std::array<char, 2> Position; 

vector<Position> Knight::getAvailPositions() const 
{ 
    vector<Position> v; 
    v.reserve(8); 
    for (char a : {2, -2}) { 
     for (char b : {1, -1}) { 
      v.emplace_back(position[0] + a, position[1] + b); 
      v.emplace_back(position[0] + b, position[1] + a); 
     } 
    } 
    removeInvalids(v); 
    return v; 
} 

bool invalid(Position p) 
{ 
    return p[0] < 'A' || p[0] > 'H' || p[1] < '0' || p[1] > '7'; 
} 

void Piece::removeInvalids(vector<Position>& v) 
{ 
    v.erase(std::remove_if(v.begin(), v.end(), invalid), v.end()); 
} 

これは、すべてのこれらの文字列& stringstreamsの内側に隠れていた動的なメモリ割り当ての絶対トンを避けることができます。今すぐあなたはgetAvailPositions()コールごとに1つしかありません。それは少ないコードです。

+0

私は主に機能性について心配していますが、パフォーマンスは実際にはこのエントリレベルのC++コースのデータ構造では問題になりません。しかし、あなたのおかげで解決して、私はそれが私の問題を解決するかどうかを確認します。それを実装するには私をしばらくお待ちください、私はあなたにお知らせします。私は洞察にも感謝します。 –

0

時には正しいだけなのは、現在のベクトル要素を最後の要素で置き換えてから次の要素に移動するためです。ベクトルの最後の要素が無効だった場合は、無効な部分をベクトルの中央に滑らせてスキップしていただけです。

あなたの例では、@ 4を削除すると?3をベクトルの真ん中に引っ張り、それからさらに質問することなく、b0を見るように移動します。コードをそのまま使用したい場合は、ベクターの途中から要素を取り除くというロジックを取り直してください。

他の回答からも指摘されているように、このコードを書くための効率的な方法があります。それはループを再開するために

void Piece::removeInvalids(vector<string>& v) 
{ 
    for (short i = 0; i < v.size(); i++) 
    { 
     string s = v.at(i); 
     size_t found = s.find_first_not_of(valids); 
     if (found != string::npos) 
     { 
      cout << v.at(i) << endl; 
      swap(v.at(i), v.back()); 
      v.pop_back(); 
      i = 0; 
     } 
    } 
} 

0

D.Rあなたは私のコードについて正しい、これはこの1行を追加することで問題を解決するようです。私は6月2日までにこのプロジェクトを終え、それを忠実に文書化するために、私は今のところこのようにしておくつもりです。私は先生がクラスへの学習経験として説明してくれると確信していますが、パフォーマンスについてはまだ深くは話していませんが、それはC++で取る次のコースの焦点になります。私はCSC-17Cを取ってパフォーマンスをチューニングするときにこのプログラムを変更できます。

ありがとうございました。

関連する問題