0

First of all, I am nothing but new to both programming and Stack Overflow.

I am self-studying with Schaum's outline for Programming with C++ and I have some issues with problem 8.24 (solutions are given to almost every problem in the book, but I want to know why my code in particular isn't working as expected).

You are supposed to be given a c-string and return the given string, but with all its tokens in reverse order (but keeping the natural order of the token itself). That is, given "Enter a sentence" it would show on screen "sentence a Enter".

My code is the following:

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

int main()
{ 
  char line1[100];
  cout << "Enter a sentence (enter \".\" to terminate input):\n";
  cin.getline(line1,100,'.');
  char line2[strlen(line1) + 1]; // we add 1 for the empty char that ends every c string
  int char_count = strlen(line1); //strlen() does not include the empty char
  char* p = strtok(line1," ");
  while (p)
  { 
    char_count -= strlen(p);  // we substract p's len to start adding its chars
    for (int i = 0; i <= strlen(p); i++)
      line2[char_count + i] = p[i]; // we then add the chars themselves
    if ((char_count - 1) > 0) 
      line2[--char_count] = ' '; // a blanck space is needed between the different tokens
    p = strtok(NULL, " ");
  }
  cout << "\n" << line2 << "\n";
}
norok2
  • 25,683
  • 4
  • 73
  • 99
  • 6
    Since you are a beginner, skip the chapter on C-Strings or Character Arrays. Proceed directly to the chapter on `std::string`. You can get very far without using character arrays. – Thomas Matthews Oct 18 '20 at 17:49
  • 3
    `strtok(line1," ")` replaces that space with terminating NUL character. Afterwards, `strlen(line1)` would compute the length of the first word only, not the whole line. – Igor Tandetnik Oct 18 '20 at 17:49
  • 2
    The book might say C++ in tthe title, but 90% of that code is C. It's too common for students to think they are learning C++ but instead they are being taught C with a sprinkling of C++. Suggest you get a better [text book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) – john Oct 18 '20 at 18:15
  • 1
    This line, `char line2[strlen(line1) + 1];`, is not C++. It's common in C and known as a VLA (variable length array). It's only a C++ language extension on some compilers. Avoid it when using C++. – doug Oct 18 '20 at 18:52

3 Answers3

0

Unfortunately, the code is wrong in many ways. The most obvious thing is the obscurity of the word reversal process (and the fact it is mixed with word iteration).

According to the commenters, you are not using C++. In C++ it would be rather straightforward:

#include <algorithm>
#include <iostream>
#include <string>

void reverse_words(std::string& s) {
   /* starting position of the word */
   size_t last_pos = 0;

   do {
       /* find the end of current word */
       size_t end_pos = std::min( s.find(' ', last_pos + 1), s.size() );
       /* reverse one word inplace */
       std::reverse(s.begin() + last_pos, s.begin() + end_pos);
       /* advance to the begining of the next word */
       last_pos = end_pos + 1;
   } while (pos != std::string::npos);

   std::reverse(s.begin(), s.end());
}

int main()
{
    std::string s = "This is a sentence";
    reverse_words(s);
    std::cout << s << std::endl;
}

Hopefully, you can see the essence of the method: sequentially find start and finish of each word, reverse letter order in this word and then finally reverse the entire string.

Now, getting back to the C-string question. You can replace std::string::find call with strtok and write your version of std::reverse specialized for C strings (the reversal of the entire string or its part is simpler than reversing the word order and this is also the recommended exercise).

Start from a simpler program which prints out pairs of integers (start_pos and end_pos for each word) using strtok. Then write a reverse procedure and test it also. Finally, combine this word iteration with reverse. I personally think this is the only way to be sure your implementation is correct - being sure in each of its parts and being able to test each part individually.

Viktor Latypov
  • 14,289
  • 3
  • 40
  • 55
0

A lot of improvements have been added to C++ since that book was originally written, and we can do it in a lot cleaner and safer way now. We'll break the problem into two parts:

  • A function to convert a string into a list of tokens
  • The main function, which reads the string; reverses it; and prints it.

These functions will be tokenize, which returns a vector of string_view, and main. A string_view is just a class that stores a pointer and a size to some other string. It's efficient because it won't make a copy of the string or allocate any memory. In this case, it's the right tool for the job because we're going to be breaking up an existing string.

#include <string_view>
#include <string>
#include <iostream>
#include <vector>
#include <algorithm>

auto tokenize(std::string_view line) {
    std::vector<std::string_view> tokens;

    for (size_t token_size = line.find(' ');
         token_size != line.npos;
         token_size = line.find(' ')) 
    {
        tokens.push_back(line.substr(0, token_size));
        line.remove_prefix(token_size + 1);
    }
    tokens.push_back(line);
    return tokens;
}

int main() {
    std::string line;
    std::getline(std::cin, line);
    auto tokens = tokenize(line);
    std::reverse(tokens.begin(), tokens.end());
    for(auto token : tokens) {
        std::cout << token << ' ';
    }
    std::cout << std::endl;
}

Explaining tokenize

Tokenize takes a string_view as input, and returns a list of the tokens. line.find(' ') will look for a space. If it finds one, it'll return the position of the space; otherwise, it'll return line.npos (which is basically the biggest possible size).

For every token we find, we

  • get the token via view.substr(0, token_size)
  • Add the token to the vector via tokens.push_back

Then, we update the line by removing the first token and the corresponding space. This is line.remove_prefix(token_size + 1);

Once there are no more spaces, we'll add the remainder of the line to the vector using tokenize.push_back(line);, and then we'll return the vector of tokens.

Explaining main

We can get the line via std::getline(std::cin, line);, which will read a line from cin and put it in the variable we give it (line). After that, we can read all the tokens in the line using the tokenize function we wrote. We'll reverse the vector of tokens via std::reverse, and then we'll print out all the tokens.

Alecto Irene Perez
  • 10,321
  • 23
  • 46
0

Thanks to each of you. Seeing your answers I have learnt quite a lot about good programming (both regarding syntax and original ways to solve the problem itself, as Viktor's). I apologise if I have not given the proper feedback, but again I am (still) unfamiliar with Stack's customs and ''policies''.