2

上周我得到了一个写一个函数的作业：该函数获取一个string和一个char值，并且应该将字符串分成两部分，在第一次出现现有字符之前和之后。

该代码有效，但我的老师告诉我再做一次，因为它的代码写得不好。但我不明白如何让它变得更好。到目前为止，我知道用空格定义两个字符串并不好，但否则我会超出范围异常。由于字符串输入发生变化，因此字符串大小每次都会发生变化。

#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;
}
4

5 回答 5

1

正如您所了解的，这些声明

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

没有意义，因为在对象中输入的字符串text基本上可以超过两个初始化的字符串。在这种情况下，使用 string 方法at可能会导致抛出异常，或者字符串将有尾随空格。

从分配的描述中，不清楚搜索的字符是否应该包含在字符串之一中。您假设该字符应包含在第二个字符串中。

考虑到参数text应该声明为常量引用。

另外，不要使用循环，最好使用类的方法，std::string例如find.

该功能可以如下所示

#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";
}

程序输出为

"Hello World"
"Hello"
" World"

正如您所看到的，分隔字符包含在第二个字符串中，尽管我认为将它从两个字符串中排除可能会更好。

另一种我认为更清晰的方法可以如下所示

#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";
}
于 2017-01-15T15:58:42.027 回答
1

为什么你的老师是对的？

您需要用空白空间初始化目标字符串的事实是可怕的：

  • 如果输入字符串较长，则会出现超出范围的错误。
  • 如果它更短，则您得到错误答案，因为在 IT 和编程中，"It works " 与"It works".

此外，您的代码不符合规范。它应该一直工作，与存储在输出字符串中的当前值无关。

备选方案 1：您的代码但可以工作

只需在开头清除目标字符串即可。然后像你一样迭代，但使用+=orpush_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;
        }
    }
}

为了避免多重索引，我使用了临时c而不是text.at(i)ortext\[i\]但这并不是真正需要的：现在，优化编译器应该产生等效的代码，无论你在这里使用什么变体。

备选方案 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); 
}
于 2017-01-15T16:05:26.643 回答
1

我建议你，学习使用 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);
}

在这里，我使用了您可以从这里和也std::find阅读有关它的信息。Iterators

你还有其他一些错误。您正在按值传递文本，每次调用函数时都会进行复制。通过引用传递它，但用它限定它将const表明它是输入参数而不是输出。

于 2017-01-15T15:37:25.733 回答
1

只有在 part1.length() 中找到该字符时，您的代码才会起作用。你需要类似的东西：

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);
    }

}

于 2017-01-15T15:38:34.300 回答
0

我看到的最大问题是您at在应该使用的地方使用push_back. 见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[i]而不是text.at(i).

于 2017-01-15T15:31:18.073 回答