String Reversal

Question

Wrote a practice code for reversing a string say. Input = "This is funny" Expected output = "ynnuf si siht"

This code works for me. Want to know your comments and a better optimised way to perform this.

void reverse_string(std::string input)
{   
    int N = input.length();
    char buffer[100];   
    char* ptr1;
    char* ptr2;
    ptr1= ptr2 = const_cast<char*>(input.c_str());
    while(*ptr2){ptr2++;}
    ptr2--;
    int i =0;
    while(ptr2 >= ptr1)
    {
        buffer[i] = *ptr2;
        ptr2--;
        i++;
    }
    buffer[i+1]='\0';
    for(int j=0;j<N;j++)
    {
        printf("%c",buffer[j]);
    }
    std::cout << endl;
}

Answer 1

You don't actually have to physically reverse the string in order to print it reversed.

#include <iostream>

void print_reversed_string(std::string s) {
    const char *begin = s.c_str();
    for (const char *current = begin + s.length() - 1; begin <= current; --current) {
        std::cout << *current;
    }

    std::cout << std::endl;
}

int main() {
    print_reversed_string("This is funny.");
}

This reverses the string without swapping any values in memory.

I wouldn't be surprised if std::reverse (as used in this answer) did something like this behind the scenes. If so, that is likely to be better optimized and more generally applicable than this solution.

Separating reversing and printing would often be good, although it simply won't work with this solution. Another reason why a compiler-optimized version of std::reverse would be better.

Note that I renamed the function to match what your original function did rather than what it said.

Answer 2

Avoid reinventing the wheel by using standard algorithms, such as std::reverse. Concretely, you could just do:

#include <iostream>
#include <string>
#include <iterator>

int main()
{
    std::string s = "this is funny";

    // Print it reversed, but don't modify s
    std::copy(s.rbegin(), s.rend(), std::ostream_iterator<char>(std::cout));
    std::cout << "\n";

    // Reverse s, and then print it
    std::reverse(s.begin(), s.end());
    std::cout << s << "\n";
}

Answer 3

Repeated your code then added some comments

void reverse_string(std::string input)

Arguably if you don't intend to alter input, you should make it const

{   
     int N = input.length();
     char buffer[100];   

What if someone passes a string of 100 or more characters?

     char* ptr1;
     char* ptr2;
     ptr1= ptr2 = const_cast<char*>(input.c_str());

• You should not be doing a const_cast here. That indicates something dangerous is going on and it is quite wrong in this situation. The char *s should be char const *.

• In c++ it's generally better practice to declare variables only when you need them, and initialise at the point of declaration. N and buffer are not used till quite some way down.

   while(*ptr2){ptr2++;}
  

Add spaces for readability. while (*ptr2) { ptr2++; }

     ptr2--;

What is wrong with char const *ptr2 = ptr1 + strlen(ptr1) - 1; or are you forbidden to use the standard library? Also (pedantically) a std::string may contain multiple 0 bytes, in which case this would only reverse the first part of it.

     int i =0;
     while(ptr2 >= ptr1)

I'd have put a space after the while here, as it isn't a function call. But that's picky.

     {
         buffer[i] = *ptr2;
         ptr2--;
         i++;

It's more usual in c++ to use pre-increment and pre-decrement.

     }
     buffer[i+1]='\0';

N should be declared here, not waaaay up there (though given the performance of string::length it's probably not worth having a separate variable for it).

     for(int j=0;j<N;j++)
     {
         printf("%c",buffer[j]);
     }

Given buffer is a null terminated string you can just use puts to output it. But see below

     std::cout << endl;

• For consistency with the rest of the code, that should be std::endl;
• Using endl is arguable. It flushes the output stream for you and this can be a serious performance hit. In any case "\n" is a lot less typing than std::endl;
• Why mix C and C++ output methods? It'd be clearer to write one of
  • std::cout << buffer << "\n"; or
  • printf("%s\n", buffer).

}

Answer 4

Looking only at the code to reverse the string, I think I'd use a somewhat different approach (than either the question or any of the answers I see right now use).

std::string reverse(std::string const &s) { 
    return std::string(s.rbegin(), s.rend());
}

As to the rest of the code: I'd much rather not see something named reverse_string not only reversing the string that it's passed, but also printing out the result (and not providing the result in any other way either). If that's what you really want, you should at least rename the function to something like print_reversed.

As a rule I'd rather keep the two separate though--it's much easier to re-use functionality when each piece only does one thing.

I'd also generally avoid using printf in C++. If you want to use a C output function, you'd probably be better off with putchar. Using the function as I've written it above, the obvious step would be something like:

std::cout << reverse("an input string");

I'd also advise avoiding using std::endl. If you just want to write a new-line, do exactly that: std::cout << "\n";. If you really want t flush the buffer (which endl does in addition to writing a new-line) do that explicitly:

std::cout << "\n" << std::flush;