BouncingCzech
BouncingCzech

Reputation: 103

Simple Sentence Reverser in C++

I'm trying to build a program to solve a problem in a text book I bought recently and it's just driving me crazy.

I have to built a sentence reverser so I get the following:

Input = "Do or do not, there is no try." Output = "try. no is there not, do or Do"

Here's what I've got so far:

void ReverseString::reversalOperation(char str[]) {
    char* buffer;
    int stringReadPos, wordReadPos, writePos = 0;

    // Position of the last character is length -1
    stringReadPos = strlen(str) - 1;
    buffer = new char[stringReadPos+1];

    while (stringReadPos >= 0) {
        if (str[stringReadPos] == ' ') {
            wordReadPos = stringReadPos + 1;
            buffer[writePos++] = str[stringReadPos--];
            while (str[wordReadPos] != ' ') {
                buffer[writePos] = str[wordReadPos];
                writePos++;
                wordReadPos++;
            }
        } else {
            stringReadPos--;
        }
    }

    cout << str << endl;
    cout << buffer << endl;
}

I was sure I was on the right track but all I get for an output is the very first word ("try.") I've been staring at this code so long I can't make any headway. Initially I was checking in the inner while look for a '/0' character as well but it didn't seem to like that so I took it out.

Upvotes: 4

Views: 479

Answers (7)

konrad.kruczynski
konrad.kruczynski

Reputation: 47661

What where the problems with your code and what are more cplusplusish ways of doing is yet well written. I would, however, like to add that the methodology

  • write a function/program to implement algorithm;
  • see if it works;
  • if it doesn't, look at code until you get where the problem is

is not too productive. What can help you resolve this problem here and many other problems in the future is the debugger (and poor man's debugger printf). It will make you able to see how your program actually works in steps, what happens to the data etc. In other words, you will be able to see which parts of it works as you expect and which behaves differently. If you're on *nix, don't hesitate to try gdb.

Upvotes: 1

Tom Kerr
Tom Kerr

Reputation: 10730

Here is a more C++ version. Though I think the simplicity is more important than style in this instance. The basic algorithm is simple enough, reverse the words then reverse the whole string.

You could write C code that was just as evident as the C++ version. I don't think it's necessarily wrong to write code that isn't ostentatiously C++ here.

void word_reverse(std::string &val) {
    size_t b = 0;
    for (size_t i = 0; i < val.size(); i++) {
        if (val[i] == ' ') {
            std::reverse(&val[b], &val[b]+(i - b));
            b = ++i;
        }
    }
    std::reverse(&val[b], &val[b]+(val.size() - b));
    std::reverse(&val[0], &val[0]+val.size());
}

TEST(basic) {
    std::string o = "Do or do not, there is no try.";
    std::string e = "try. no is there not, do or Do";
    std::string a = o;
    word_reverse(a);
    CHECK_EQUAL( e , a );
}

Having a multiple, leading, or trailing spaces may be degenerate cases depending on how you actually want them to behave.

Upvotes: 0

JoeFish
JoeFish

Reputation: 3100

In the interest of science, I tried to make your code work as is. Yeah, it's not really the C++ way to do things, but instructive nonetheless.

Of course this is only one of a million ways to get the job done. I'll leave it as an exercise for you to remove the trailing space this code leaves in the output ;)

I commented my changes with "EDIT".

char* buffer;
int stringReadPos, wordReadPos, writePos = 0;

// Position of the last character is length -1
stringReadPos = strlen(str) - 1;
buffer = new char[stringReadPos+1];

while (stringReadPos >= 0) {
    if ((str[stringReadPos] == ' ') 
            || (stringReadPos == 0)) // EDIT: Need to check for hitting the beginning of the string
            {
        wordReadPos = stringReadPos + (stringReadPos ? 1 : 0); // EDIT: In the case we hit the beginning of the string, don't skip past the space

        //buffer[writePos++] = str[stringReadPos--]; // EDIT: This is just to grab the space - don't need it here
        while ((str[wordReadPos] != ' ')
               && (str[wordReadPos] != '\0'))  // EDIT: Need to check for hitting the end of the string
               {
            buffer[writePos] = str[wordReadPos];
            writePos++;
            wordReadPos++;
        }
        buffer[writePos++] = ' '; // EDIT: Add a space after words
    }

    stringReadPos--; // EDIT: Decrement the read pos every time        
}
buffer[writePos] = '\0'; // EDIT: nul-terminate the string

cout << str << endl;
cout << buffer << endl;

Upvotes: 2

Paolo Brandoli
Paolo Brandoli

Reputation: 4728

I see the following errors in your code:

  • the last char of buffer is not set to 0 (this will cause a failure in cout<
  • in the inner loop you have to check for str[wordReadPos] != ' ' && str[wordReadPos] != 0 otherwise while scanning the first word it will never find the terminating space

Upvotes: 2

K-ballo
K-ballo

Reputation: 81409

This is utter easy in C++, with help from the standard library:

std::vector< std::string > sentence;
std::istringstream input( str );

// copy each word from input to sentence
std::copy(
    (std::istream_iterator< std::string >( input )), std::istream_iterator< std::string >()
  , std::back_inserter( sentence )
);
// print to cout sentence in reverse order, separated by space
std::copy(
    sentence.rbegin(), sentence.rend()
  , (std::ostream_iterator< std::string >( std::cout, " " ))
);

Upvotes: 5

Jerry Coffin
Jerry Coffin

Reputation: 490663

Unless you're feeling masochistic, throw your existing code away, and start with std::vector and std::string (preferably an std::vector<std::string>). Add in std::copy with the vector's rbegin and rend, and you're pretty much done.

Upvotes: 5

onit
onit

Reputation: 6376

Since you are using a char array, you can use C string library. It will be much easier if you use strtok: http://www.cplusplus.com/reference/clibrary/cstring/strtok/

It will require pointer use, but it will make your life much easier. Your delimiter will be " ".

Upvotes: 1

Related Questions