Не совсем понимаю, почему происходит лишь одна итерация цикла while (sentence)

Question

Не совсем понимаю, почему происходит лишь одна итерация цикла while (sentence)

Рейтинг: 0Ответов: 1Опубликовано: 15.03.2023

Такая проблема: дана задача написать программу, которая из введённого текста будет выделять последние слова каждого предложения. И вот для выделения слов из предложения необходимо написать пользовательскую функцию. Её написал, но теперь не совсем понимаю, в чём ошибся: происходит лишь одна итерация цикла while (sentence). То есть, функция рассматривает только одно предложение: первое, выводит из него последнее слово и заканчивает выполнение, так как цикл while (sentence) перестаёт делать итерации. Подскажите, пожалуйста, что не так.

void On_the_words(char* text, char**& words, int& k)
{
    char** sentences = nullptr;
    char* sentence = strtok(text, ".!?");
    int count = 0;
    while (sentence)
    {
        sentences = (char**)realloc(sentences, ++count * sizeof(char*));
        sentences[count - 1] = sentence;
        char* last_word = nullptr; // переменная для хранения последнего слова в предложении
        char* word = strtok(sentences[count - 1], " ,");
        while (word)
        {   last_word = (char*)realloc(last_word, count * sizeof(char));
            last_word = word; // запоминаем последнее слово перед переходом к следующему
            word = strtok(NULL, " ,");
        }
        words = (char**)realloc(words, ++k * sizeof(char*));
        words[k - 1] = last_word; // добавляем последнее слово в массив words
        sentence = strtok(NULL, ".!?");
    } 
   free(sentences);
   free(sentence);
}

c#

Источник: Stack Overflow на русском

Answer 1

▲ 0Принят

Очень всё странно! У вас же метка C++ стоит, зачем вы пишете сишный код?
Ну в принципе ладно. У вас ошибка с использованием strtok(). Почитайте как она работает. strtok()

void On_the_words(char* text, char**& words, int& k)
{   // для примера входная строка "one, two! three, four?\0"
    char* sentence = strtok(text, ".!?");
    // sentence = "one, two!" причем это указатель на первоначальную строку, а не копия!
    // строка стала такой "one, two\0 three, four?\0"
    while (sentence)
    {
        sentences[count - 1] = sentence;
        // на вход подается "one, two\0"
        char* word = strtok(sentences[count - 1], " ,"); 
        // word = "one" sentences[count - 1] = "one\0 two\0"
        while (word)
        {   
            last_word = word; // запоминаем последнее слово перед переходом к следующему
            word = strtok(NULL, " ,");
            // word = "two" 
            // word = "\0"
        }
        // а вот здесь strtok() работает с указателем на "\0", оставшимся после последнего вызова
        sentence = strtok(NULL, ".!?");
        // результат  sentence = nullptr
    } 
}

После внутреннего цикла, внутренний указатель функции strtok() указывает на ноль завершающий строку "one, two\0". Соответственно поиск следующего токена ничего не находит и возвращается nullptr. Просто нельзя пересекать вызовы strtok() - сначала нужно получить указатели на все sentence = strtok(text, ".!?");, а только потом пройти по каждому из них и разбить на слова word = strtok(NULL, " ,");.
Причем использование realloc() под массив указателей - не самая удачная идея. Воспользуйтесь простым массивом указателей. Или сделайте vector<char*> - гораздо эффективнее по быстродействию будет, особенно на больших текстах.
С realloc() у вас ещё одна неочевидная ошибка - если она не сможет выделить память, то старый блок не удалит, а просто вернет nullptr. В результате будет сбой программы. И вообще выделять/перевыделять память вручную в C++ - плохой стиль. Почитайте про RAII.

sentences = (char**)realloc(sentences, ++count * sizeof(char*)); // sentences == nullptr !!!

// надо так
char** tmp = (char**)realloc( .... );
if(tmp)
    sentences = tmp;
else
  // что-то делать с ошибкой.

Но это всё здесь вообще лишнее - sentences = (char**)realloc(sentences, ++count * sizeof(char*)); Зачем вы запоминаете строки, если их дальше нигде не используете? Вам нужна одна текущая строка, достаточно одного указателя, а не массива. Также last_word это указатель на одну строку. Зачем на него перевыделять память, тем более она утекает. И вы ещё получаете UB когда попытаетесь перевыделить память, которую не выделяли.

while (word)
{   last_word = (char*)realloc(last_word, count * sizeof(char));
    last_word = word; // утечка памяти
                      // а на следующей итерации будет UB
}

Внутренний цикл разбиения на токены можно заменить на поиск с конца строки. На больших строках даст хорошую экономию по производительности!

//    while (sentence)  // вместо цикла - поиск с конца
    {
//        sentences = (char**)realloc(sentences, ++count * sizeof(char*)); // не нужно
//        sentences[count - 1] = sentence;  // не нужно
        char* last_word = strrchr(sentence, ' '); // сделать поиск с конца
        words = (char**)realloc(words, ++k * sizeof(char*)); // лучше заменить на vector<char*>
        words[k - 1] = last_word; // добавляем последнее слово в массив words
        sentence = strtok(NULL, ".!?");
    }

Вообще всё то же самое можно написать на std::string_view - никаких копирований, просто работа с диапазонами исходной строки. А в векторе есть размер, поэтому отдельно его передавать не надо.

void 
On_the_words(string_view &text, vector<string_view> &words)
{
    while ( !text.empty() )
    {
        size_t pos = text.find_first_of(".!?"sv);
        string_view sentence = text.substr( 0, pos);
        text = text.substr(pos+1);

        pos = sentence.find_last_of(" ,"sv); // сделать поиск с конца
        string_view last_word = sentence.substr( pos+1 );
        words.push_back(last_word);
    } 
}
    
int main()
{
    string_view str("one, two! three, four?");
    vector<string_view> words;
    On_the_words( str, words);

    cout << str << "\n";
    for(auto i : words)
        cout << i << "\n";

    return 0;
}

Не совсем понимаю, почему происходит лишь одна итерация цикла while (sentence)

Ответы