Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###Comments on code

Comments on code

In C++ code you do not want to be messing with C-Strings. All that memory management makes the code hard to maintain and is difficult to get correct. prefer to use std::string which does all the hard work and lets you concentrate on the algorithm.

void reverse_word(char* s, char* e)

There are already algorithms for reversing stuff std::reverse() and swapping the two values std::swap() so there is no need to implement either.

 while(s < e)
 {
 char tmp = *s;
 *s = *e;
 *e = tmp;
 ++s;
 --e;
 }

Space ' ' is not the only white space character. Rather than explicitly testing for a space you should use the standard function for testing if a character is white space std::is_space()

 while((*p != '0円') && (*p == ' '))
 

###Comments on Algorithm

Comments on Algorithm

There is already a std::reverse() algorithm.
Using this would reduce the complexity of your code to just finding the beginning and end of each word. Since operator>> does this automatically it makes the code very trivial:

void reverse_string(std::string& paragraph)
{
 std::stringstream pStream(paragraph);
 std::string word;
 paragraph.clear();
 while(pStream >> word)
 {
 std::reverse(word.begin(), word.end());
 paragraph.append(word);
 paragraph.append(" ");
 }
}

###Comments on code

In C++ code you do not want to be messing with C-Strings. All that memory management makes the code hard to maintain and is difficult to get correct. prefer to use std::string which does all the hard work and lets you concentrate on the algorithm.

void reverse_word(char* s, char* e)

There are already algorithms for reversing stuff std::reverse() and swapping the two values std::swap() so there is no need to implement either.

 while(s < e)
 {
 char tmp = *s;
 *s = *e;
 *e = tmp;
 ++s;
 --e;
 }

Space ' ' is not the only white space character. Rather than explicitly testing for a space you should use the standard function for testing if a character is white space std::is_space()

 while((*p != '0円') && (*p == ' '))
 

###Comments on Algorithm

There is already a std::reverse() algorithm.
Using this would reduce the complexity of your code to just finding the beginning and end of each word. Since operator>> does this automatically it makes the code very trivial:

void reverse_string(std::string& paragraph)
{
 std::stringstream pStream(paragraph);
 std::string word;
 paragraph.clear();
 while(pStream >> word)
 {
 std::reverse(word.begin(), word.end());
 paragraph.append(word);
 paragraph.append(" ");
 }
}

Comments on code

In C++ code you do not want to be messing with C-Strings. All that memory management makes the code hard to maintain and is difficult to get correct. prefer to use std::string which does all the hard work and lets you concentrate on the algorithm.

void reverse_word(char* s, char* e)

There are already algorithms for reversing stuff std::reverse() and swapping the two values std::swap() so there is no need to implement either.

 while(s < e)
 {
 char tmp = *s;
 *s = *e;
 *e = tmp;
 ++s;
 --e;
 }

Space ' ' is not the only white space character. Rather than explicitly testing for a space you should use the standard function for testing if a character is white space std::is_space()

 while((*p != '0円') && (*p == ' '))
 

Comments on Algorithm

There is already a std::reverse() algorithm.
Using this would reduce the complexity of your code to just finding the beginning and end of each word. Since operator>> does this automatically it makes the code very trivial:

void reverse_string(std::string& paragraph)
{
 std::stringstream pStream(paragraph);
 std::string word;
 paragraph.clear();
 while(pStream >> word)
 {
 std::reverse(word.begin(), word.end());
 paragraph.append(word);
 paragraph.append(" ");
 }
}
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

###Comments on code

In C++ code you do not want to be messing with C-Strings. All that memory management makes the code hard to maintain and is difficult to get correct. prefer to use std::string which does all the hard work and lets you concentrate on the algorithm.

void reverse_word(char* s, char* e)

There are already algorithms for reversing stuff std::reverse() and swapping the two values std::swap() so there is no need to implement either.

 while(s < e)
 {
 char tmp = *s;
 *s = *e;
 *e = tmp;
 ++s;
 --e;
 }

Space ' ' is not the only white space character. Rather than explicitly testing for a space you should use the standard function for testing if a character is white space std::is_space()

 while((*p != '0円') && (*p == ' '))
 

###Comments on Algorithm

There is already a std::reverse() algorithm.
Using this would reduce the complexity of your code to just finding the beginning and end of each word. Since operator>> does this automatically it makes the code very trivial:

void reverse_string(std::string& paragraph)
{
 std::stringstream pStream(paragraph);
 std::string word;
 paragraph.clear();
 while(pStream >> word)
 {
 std::reverse(word.begin(), word.end());
 paragraph.append(word);
 paragraph.append(" ");
 }
}
lang-c

AltStyle によって変換されたページ (->オリジナル) /