String reversal function

Question

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");
}

Answer 1

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.

• einpoklum: 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.
• Artemy Vysotsky: @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.
• Rakete1111: @einpoklum The size of the string in OP's code is hard coded. With std::string, you don't have that "problem".

Answer 2

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.

Answer 3

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);
}