Reverse a word using string manipulation

#include
#include
#include
#include
#include
using namespace std;
string temp[10];
void extract(char * c)
{
    int ind = 0;

    //char *temp = (char *)malloc(sizeof(char));
    while(*c!= NULL)
    {
        if(*c!= ' ') // read all chars in a word and store in temp.
        {
        temp[ind]= temp[ind] + *c;
        c++;

        }
        else if(*c== ' ') // reached end of word
        {
        c++;
        ind++;
        }
    }
    int length = sizeof(temp)/sizeof(temp[0]);
    printf("The reversed string is");
    for(int i=length;i>=0;i--)
    {
    cout<<temp[i];// not able to print it with printf statement
    }

}

void main()
{
char *c = "seattle is good";
extract(c);

}

This should print the sentence in reverse order . Input is "seattle is good"
Output for the above will be "good is seattle"
I have 2 questions ,

1. I am using string array here with size of 10. How can I make it dynamic?
2. For some reason the printf("%s",temp[i]) was not giving me correct results(At the very end of extract method). I then used cout and it was fine. Any ideas why? Am I doing something silly?

Algorithm Description:

• Take the input string as char array.

• Extract words from the string by looking for empty space.

• Store each word in a temp string array.

temp[0] = seattle
temp[1] = is
temp[2] = good

• Then loop backwards through this temp array and print contents.

In response to your specific questions:

• Use a vector. Using a vector you can just append to the end and it will resize itself as necessary.

• You can not use printf with strings as printf is a C function and does not know C++'s strings. Either way you should generally avoid printf in C++ code.

General notes on your code:

#include
#include
#include

The correct way to include C header files in C++ is to use #include , not #include . Further you should avoid C-functions when there are good C++ alternatives and you should generally not mix C's stdio with C++'s iostream. In this particular program I do not believe you need any of the C headers you include.

string temp[10];

I don't see any reason to have your global storage as a global variable. You should move it as a local variable inside the extract method. And as I already said, you should use a vector instead of a C array.

Also temp is a bad variable name.

void extract(char * c)

Don't mix char*s and strings unless necessary. In this case you can use strings all the way, so you should define extract with const string& sentence as an argument (note that sentence is also a more descriptive name than c).

Further it seems that your extract function is doing too much work: it's extracting words from the sentence into an array, and then printing it in reverse. These two separate steps should be done by two functions: extract (or maybe sentence_to_words which is more descriptive) and print_in_reverse.

You might even want to use 3 steps instead: extract the words, reverse them, then print them. This way your IO code is completely separate from any logic, which is always good. As a bonus point C++ already has a built-in function to reverse an array or vector, so no additional work for you.

Either way you'd need to change your extract function to either return the extracted words rather than void or take a vector into which to store the extracted words as an argument (by reference).

//char *temp = (char *)malloc(sizeof(char));

This code is commented out anyway (and doesn't do anything sensible from the looks of it), but as a general principle I want to point out that there's rarely a reason to use malloc over new in C++.

int ind = 0;
while(*c!= NULL)
{
    if(*c!= ' ') // read all chars in a word and store in temp.
    {
    temp[ind]= temp[ind] + *c;
    c++;

    }
    else if(*c== ' ') // reached end of word
    {
    c++;
    ind++;
    }
}

Using a vector you can get rid of ind as you can just push_back into the vector.

Further if you use strings, you can simplify this part by using find to find the next space and substr take the substring up to that space.

int length = sizeof(temp)/sizeof(temp[0]);

Note that this will only ever tell you the size of an array if the array's size is known at compile time (which is more or less equivalent to: "spelled out in the code"). So if you want the size of the array to be dynamic, you can't use this. If you use a vector, you can just use temp.size().

printf("The reversed string is");

As I already said, you shouldn't mix stdio and iostream. There is no reason to use printf here.

void main()

The correct return value for main is int, not void.

char *c = "seattle is good";

Again this should just be string sentence("seattle is good");. No reason to use char*.

If I were to write this program, I might do it like this:

#include 
#include 
#include 
#include 
#include 
#include 

using std::string;
using std::vector;
using std::cout;
using std::endl;
using std::reverse;

// Splits a sentence by spaces into a vector of words
void extract(const string& sentence, vector& words)
{
    size_t pos = 0;
    while(pos != string::npos)
    {  
        // Find position of next space
        int next_space = sentence.find(" ", pos);
        // Store everything up to next space in the words vector
        words.push_back( sentence.substr(pos, next_space - pos));

        // Continue at the next character which is not a space.
        pos = sentence.find_first_not_of(" ", next_space);
    }
}

// Prints the strings in the vector separated by spaces
void print_strings(const vector& strings)
{
    for(size_t i = 0; i  words;
    extract(sentence, words);
    reverse(words.begin(), words.end());
    print_strings(words);
}

StackExchange Code Review Q#1254, answer score: 12