14

I wrote a simple C++ program to reverse a string. I store a string in character array. To reverse a string I am using same character array and temp variable to swap the characters of an array.

#include<iostream>
#include<string>
using namespace std;

void reverseChar(char* str);

char str[50],rstr[50];
int i,n;

int main()
{
    cout<<"Please Enter the String: ";
    cin.getline(str,50);
    reverseChar(str);
    cout<<str;
    return 0;
}

void reverseChar(char* str)
{
    for(i=0;i<sizeof(str)/2;i++)
    {
        char temp=str[i];
        str[i]=str[sizeof(str)-i-1];
        str[sizeof(str)-i-1]=temp;
    }
}

Now this method is not working and, I am getting the NULL String as result after the program execution.

So I want to know why I can't equate character array, why wouldn't this program work. And what is the solution or trick that I can use to make the same program work?

Eric Leschinski
  • 146,994
  • 96
  • 417
  • 335
StackPointer
  • 529
  • 2
  • 7
  • 18

5 Answers5

48

sizeof(str) does not do what you expect.

Given a char *str, sizeof(str) will not give you the length of that string. Instead, it will give you the number of bytes that a pointer occupies. You are probably looking for strlen() instead.

If we fixed that, we would have:

for(i=0;i<strlen(str)/2;i++)
{
    char temp=str[i];
    str[i]=str[strlen(str)-i-1];
    str[strlen(str)-i-1]=temp;
}

This is C++, use std::swap()

In C++, if you want to swap the contents of two variables, use std::swap instead of the temporary variable.

So instead of:

char temp=str[i];
str[i]=str[strlen(str)-i-1];
str[strlen(str)-i-1]=temp;

You would just write:

swap(str[i], str[sizeof(str) - i - 1]);

Note how much clearer that is.

You're using C++, just use std::reverse()

std::reverse(str, str + strlen(str));

Global variables

It's extremely poor practice to make variables global if they don't need to be. In particular, I'm referring to i about this.

Executive Summary

If I was to write this function, it would look like one of the two following implementations:

void reverseChar(char* str) {
    const size_t len = strlen(str);

    for(size_t i=0; i<len/2; i++)
        swap(str[i], str[len-i-1]);
}

void reverseChar(char* str) {
    std::reverse(str, str + strlen(str));
}

When tested, both of these produce dlrow olleh on an input of hello world.

Bill Lynch
  • 80,138
  • 16
  • 128
  • 173
  • 1
    In your example, are you using the false `sizeof` again? – poitroae Jun 23 '14 at 18:22
  • @aoeu: Yeah. I wasn't sure if I should have both the fix about using std::swap and the bug about sizeof in there, or to leave them separate. I've added some more content to have the fixes cascade more legibly. – Bill Lynch Jun 23 '14 at 18:23
  • Thank You! Worked! So sizeof is for pointer purpose only? Or can I use for the array as well? – StackPointer Jun 23 '14 at 18:25
  • 1
    @StackPointer: Read these: http://stackoverflow.com/questions/3203162/what-does-sizeof-do http://en.cppreference.com/w/cpp/language/sizeof – Bill Lynch Jun 23 '14 at 18:27
1

The problem is that within your function, str is not an array but a pointer. So sizeof will get you the size of the pointer, not the length of the array it points to. Also, even if it gave you the size of the array, that is not the length of the string. For this, better use strlen.

To avoid multiple calls to strlen, give the function another parameter, which tells the length:

void reverseChar(char* str, int len)
{
    for(i=0; i<len/2; i++)
    {
        char temp=str[i];
        str[i]=str[len-i-1];
        str[len-i-1]=temp;
    }
}

and call it with

reverseChar(str, strlen(str))

Another improvement, as mentioned in the comments, is to use std::swap in the loop body:

void reverseChar(char* str, int len)
{
    for(i=0; i<len/2; i++)
    {
        std::swap(str[i], str[len-i-1]);
    }
}

Also, there is std::reverse which does almost exactly that.

leemes
  • 44,967
  • 21
  • 135
  • 183
-1
//reverse a string
#include<iostream>
using namespace std;

int strlen(char * str) {
  int len = 0; 
  while (*str != '\0') {
    len++;
    str++;
  }
  return len;
}

void reverse(char* str, int len) {
  for(int i=0; i<len/2; i++) {
    char temp=str[i];
    str[i]=str[len-i-1];
    str[len-i-1]=temp;
  }
}

int main() {
  char str[100];
  cin.getline(str,100);
  reverse(str, strlen(str));
  cout<<str<<endl;
  getchar();
  return 0;
}
Stephen Rodriguez
  • 1,037
  • 1
  • 12
  • 22
Kartik
  • 9
  • 2
  • could you give add a short description to your answer that explains why your code is different from the others? It's unclear without trying it out, or having to play a human compiler. – Bart Jun 23 '15 at 19:39
-2

If I were you, I would just write it like so:

int main()
{
    string str;
    cout << "Enter a string: " << endl;
    getline(cin, str);
    for (int x = str.length() - 1; x > -1; x--)
    {
        cout << str[x];
    }
    return 0;
}

This is a very simple way to do it and works great.

Cjolsen06
  • 89
  • 1
  • 2
  • 6
-2
#include <iostream>
#include <cstdio>

using namespace std;

int main()
{
    char str[80];
    cout << "Enter a string bro: \n";
    gets_s(str);

    for (int i = strlen(str) - 1; i > -1; i--)
    {
        cout << str[i];
    }
}
Imalrightatthisstuff
  • 1