# String reversal function

I have an assignment to create a function that reverses the letters of each word in a string individually. I feel like this should be a rather simple task. I did figure out a solution, although I feel like it uses too many variables as a sort of hack method. Any suggestions on how to make it more efficient/shorter?

#include<iostream>
#include<conio.h>
#include<string.h>

using namespace std;

void rev(char str[80])
{
char t;
int l = 0, n = strlen(str), k = 0;

for (int i = 0; i <= n; i++)
{
if (str[i] == ' ' || str[i] == '\0')
{
for (int j = i - 1; j > (k + i - 1) / 2; l++, j--)
{
t = str[j];
str[j] = str[l];
str[l] = t;
}
k = i + 1;
l = k;
}
}

}

void main()
{
char str[80];
cout << "Please enter a string : ";
cin.getline(str, 80);
cout << "The original string is : " << str << "\n";
rev(str);
cout << "The new string is : " << str << "\n";
system("pause");
}


1. main should be int main()

2. Why C-style strings? Use std::string

3. One function - one task. You are mixing 2 things into one function:

• Reverse chars in a word
• Find words in a sentence separated by spaces

I feel like your problem with too many variables will resolve when you split the tasks you are doing into separate functions.

4. Use better code formatting (auto-format).

5. Type mixing - strlen's return value is std::size_t. Do not assign to int. As a follow-up, use a loop index of type std::size_t to match.

6. This is my preference - not using using namespace std; and system("pause");. I think a lot of posts could be found explaining why.

• Disagree about the use of std::string - this assignment is a self-contained exercise, and a sequence of characters in memory is fine; the features of std::string are not useful here. Oct 1, 2017 at 7:48
• @einpoklum assignment is worded as "letters of each word in a string". String in C++ is represented by std::string. Where did you get a sequence of characters I do not know. Oct 1, 2017 at 7:57
• @einpoklum The size of the string in OP's code is hard coded. With std::string, you don't have that "problem". Oct 1, 2017 at 8:13

I'm just going to assume you cannot use std::string at all here, otherwise the code would be much shorter for learning purposes.

• If you have to use a fixed-size C-string, then you should at least make it much larger so that the input string won't likely overflow. If that were to happen here, it would cause a buffer overflow. Otherwise, use a dynamically-allocated array to hold the correct size of the input string, while remembering to deallocate the array afterwards.

char* str = new char[80];

// reallocate and/or use as needed

delete [] str;

• I don't think you need <conio.h>, especially in C++, so just remove it.

• It's usually preferred to put a space between #include and whatever is being included.
• This is not so clear at a glance:

int l = 0, n = strlen(str), k = 0;


It's more preferred to have each variable declared or initialized on separate lines:

int l = 0;
int n = strlen(str);
int k = 0;

• Do not use single-character variable names except for loop counter variables. It could be very hard for others to tell what they're used for, and even you may end up forgetting this after a while. If you're trying to achieve shorter but still good code, then this isn't really the way to do it.
• Instead of that manual swap code, you can just use std::swap():

std::swap(str[1], str[j]);

• Do not use system("PAUSE") to pause. It's not portable and uses more resources. Instead, use something else that still acts as a pause, such as:

std::cin.get();


One difference is that this requires a character to be entered as opposed to proceeding right after the next keypress.

I suggest you separate the word reversal from the actual algorithm:

void reverse_string(char* start, char* end)
{
char tmp;

for (--end; start < end; ++start, --end)
{
tmp = *start;
*start = *end;
*end = tmp;
}
}

void reverse_words(char* str)
{
char* end_of_string = str + strlen(str);
char* start_of_word = str;
char* end_of_word = strchr(str, ' ');

while (end_of_word)
{
reverse_string(start_of_word, end_of_word);
start_of_word = end_of_word + 1;
end_of_word = strchr(start_of_word, ' ');
}

reverse_string(start_of_word, end_of_string);
}