文字列宣言/参照パラメータの難しさ（C++）

Question

先週私は関数を書くために宿題を得ました。この関数はstringとcharという値を得て、その最初の出現の前後に2つの部分で文字列を分割する必要があります既存のchar。

コードはうまくいきましたが、私の先生は私に教えてくれました。コードがよく書かれていないからです。しかし、私はそれをより良くする方法を理解していません。私は白い空白で2つの文字列を定義するのは良くないと理解していますが、そうでなければ範囲外になります。文字列入力が変わるので、文字列サイズは毎回変わります。

#include <iostream> 
#include <string> 
using namespace std; 

void divide(char search, string text, string& first_part, string& sec_part) 
{ 
    bool firstc = true; 
    int counter = 0; 

    for (int i = 0; i < text.size(); i++) { 
     if (text.at(i) != search && firstc) { 
      first_part.at(i) = text.at(i); 
     } 
     else if (text.at(i) == search&& firstc == true) { 
      firstc = false; 
      sec_part.at(counter) = text.at(i); 
     } 
     else { 
      sec_part.at(counter) = text.at(i); 
      counter++; 
     } 
    } 
} 

int main() { 
    string text; 
    string part1="       "; 
    string part2="       "; 
    char search_char; 

    cout << "Please enter text? "; 
    getline(cin, text); 

    cout << "Please enter a char: ? "; 
    cin >> search_char; 

    divide(search_char,text,aprt1,part2); 
    cout << "First string: " << part1 <<endl; 
    cout << "Second string: " << part2 << endl; 

    system("PAUSE"); 
    return 0; 
} 

Answer 1

私が見る最大の問題は、あなたがpush_backを使用する必要がありますatを使用していることです。 std::basic_string::push_backを参照してください。 atは、にアクセスして、既存の文字を読み取ったり変更したりするように設計されています。 push_backは、文字列に新しい文字を追加します。

divideは、次のようになります。

void divide(char search, string text, string& first_part, 
      string& sec_part) 
{ 
    bool firstc = true; 

    for (int i = 0; i < text.size(); i++) { 
     if (text.at(i) != search && firstc) { 
      first_part.push_back(text.at(i)); 
     } 
     else if (text.at(i) == search&& firstc == true) { 
      firstc = false; 
      sec_part.push_back(text.at(i)); 
     } 
     else { 
      sec_part.push_back(text.at(i)); 
     } 
    } 
} 

あなたが例外を処理していないので、text.at(i)ではなくtext[i]を使用することを検討してください。

Answer 2

私はあなたにお勧めします、C++標準機能の使用を学びます。プログラミングに役立つ豊富なユーティリティ機能があります。ここで

void divide(const std::string& text, char search, std::string& first_part, std::string& sec_part) 
{ 
    std::string::const_iterator pos = std::find(text.begin(), text.end(), search); 

    first_part.append(text, 0, pos - text.begin()); 
    sec_part.append(text, pos - text.begin()); 
} 

int main() 
{ 
    std::string text = "thisisfirst"; 
    char search = 'f'; 

    std::string first; 
    std::string second; 

    divide(text, search, first, second); 
} 

私はあなたがhereともIteratorsから、それについて読むことができますstd::findを使用。

その他の間違いがあります。関数を呼び出すたびにコピーを行う値でテキストを渡しています。参照として渡しますが、出力ではなく入力パラメータであることを示すconstで修飾します。

Answer 3

あなたのコードは、part1.length（）内に文字がある場合に限り動作します。あなたが自分自身を理解したようオブジェクトに入力した文字列がtextは基本的に両方の初期化を超えることがあるので、これらの宣言

string part1="       "; 
string part2="       "; 

は意味がありません

void string_split_once(const char s, const string & text, string & first, string & second) { 
    first.clear(); 
    second.clear(); 
    std::size_t pos = str.find(s); 
    if (pos != string::npos) { 
     first = text.substr(0, pos); 
     second = text.substr(pos); 
    } 

}

Answer 4

：あなたは、これに似た何かが必​​要文字列。この場合、文字列メソッドatを使用すると例外がスローされたり、文字列の末尾に空白が付きます。

割り当ての説明から、検索された文字が文字列の1つに含まれるべきかどうかは不明です。文字を2番目の文字列に含める必要があるとします。

パラメータtextは定数参照として宣言する必要があることを考慮してください。

また、ループを使用する代わりに、たとえばfindのようなクラスstd::stringのメソッドを使用する方が良いです。

機能は次のように見ることができます

私は多分それは可能だと思うけれどもあなたは分離文字が2番目の文字列に含まれて見ることができるようにプログラム出力は

"Hello World" 
"Hello" 
" World" 

ある

#include <iostream> 
#include <string> 

void divide(const std::string &text, char search, std::string &first_part, std::string &sec_part) 
{ 
    std::string::size_type pos = text.find(search); 

    first_part = text.substr(0, pos); 

    if (pos == std::string::npos) 
    { 
     sec_part.clear(); 
    } 
    else 
    { 
     sec_part = text.substr(pos); 
    } 
} 

int main() 
{ 
    std::string text("Hello World"); 
    std::string first_part; 
    std::string sec_part; 

    divide(text, ' ', first_part, sec_part); 

    std::cout << "\"" << text << "\"\n"; 
    std::cout << "\"" << first_part << "\"\n"; 
    std::cout << "\"" << sec_part << "\"\n"; 
} 

両方の文字列から除外する方がよいでしょう。

代替と私の意見より明確なアプローチでは、

#include <iostream> 
#include <string> 
#include <utility> 

std::pair<std::string, std::string> divide(const std::string &s, char c) 
{ 
    std::string::size_type pos = s.find(c); 

    return { s.substr(0, pos), pos == std::string::npos ? "" : s.substr(pos) }; 
} 

int main() 
{ 
    std::string text("Hello World"); 

    auto p = divide(text, ' '); 

    std::cout << "\"" << text << "\"\n"; 
    std::cout << "\"" << p.first << "\"\n"; 
    std::cout << "\"" << p.second << "\"\n"; 
} 

Answer 5

次のように見ることができますなぜあなたの先生は正しいでしょうか？あなたは空のスペースを使用して、宛先の文字列を初期化する必要があること

事実がひどいです：

• 入力文字列が長い場合は、バインドされたエラーの出るでしょう。
• ITとプログラミングでは、"It works "は"It works"と同じではないため、短くすれば間違った答えが得られます。

さらに、コードが仕様に適合しません。出力文字列に格納されている現在の値とは独立して、常に動作するはずです。

オルタナティブ1：あなたのコードが、

が始まったばかりで、先の文字列をクリアする作業。その後、繰り返すが、+=またはpush_back()を使用して文字列の最後に文字を追加する。

void divide(char search, string text, string& first_part, string& sec_part) 
{ 
    bool firstc = true; 
    first_part.clear(); // make destinations strings empty 
    sec_part.clear(); 

    for (int i = 0; i < text.size(); i++) { 
     char c = text.at(i); 
     if (firstc && c != search) { 
      first_part += c; 
     } 
     else if (firstc && c == search) { 
      firstc = false; 
      sec_part += c; 
     } 
     else { 
      sec_part += c; 
     } 
    } 
} 

私は、複数のインデックスを避けるために、代わりに text.at(i)または text\[i\]の一時的な cを使用しかし、これは本当に必要とされていない。最近は、最適化コンパイラは、ここで使用するものは何でもバリアント同等のコードを生成する必要があります。

代替2：使用ストリングメンバ関数

この代替は、find()関数を使用し、その位置まで開始から文字列を構築し、その位置からもう。文字が見つからなかった特別なケースがあります。

void divide(char search, string text, string& first_part, string& sec_part) 
{ 
    auto pos = text.find(search); 
    first_part = string(text, 0, pos); 
    if (pos== string::npos) 
     sec_part.clear(); 
    else sec_part = string(text, pos, string::npos); 
}