0

I am new to cpp and have a question regarding arrays. The code I have below should create a reversed version of str and have it be stored in newStr. However, newStr always comes up empty. Can someone explain to me why this is happening even though I am assigning a value from str into it?

void reverse (char* str) {
    char* newStr = (char*)malloc(sizeof(str));

    for (int i=0;i<sizeof(str)/sizeof(char);i++) {
       int index = sizeof(str)/sizeof(char)-1-i;
       newStr [i] = str [index];
    }
}

PS: I know that it is much more efficient to reverse an array by moving the pointer or by using the std::reverse function but I am interested in why the above code does not work.

Improve this question
7
  • 1
    Many dupes, but sizeof(char*) isn't the size of the array. You should also use std::string, or, if you have no choice, at least new/new[] and delete/delete[] over malloc and free. Commented Sep 24, 2012 at 0:42
  • 2
    Don't use malloc in C++, use new. Commented Sep 24, 2012 at 0:43
  • Nothing to do with unique_ptr Commented Sep 24, 2012 at 0:49
  • @Aesthete yes, he's allocating memory in a function and hoping the caller knows to deallocate the returned pointer. You need unique_ptr to fix that situation. Commented Sep 24, 2012 at 0:53
  • 2
    You don't need unique_ptr to fix that, you need a better design and a good working knowledge of memory management. Don't throw abstractions at people before they understand what they're abstracting away. Commented Sep 24, 2012 at 1:00

4 Answers 4

Reset to default
6

As above commenters pointed out sizeof(str) does not tell you the length of the string. You should use size_t len = strlen(str);

void reverse (char* str) {
   size_t len = strlen(str);
   char* newStr = (char*)malloc(len + 1);

   for (int i=0; i<len;i++) {
      int index = len-1-i;
      newStr[i] = str[index];
   }
   newStr[len] = '\0'; // Add terminator to the new string.
}

Don't forget to free any memory you malloc. I assume your function is going to return your new string?

Edit: +1 on the length to make room for the terminator.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

One bug in your code: newStr doesn't have a NULL terminating character.
@Code-Guru absolutely right. Updated the answer with your feedback.
The NULL character won't be copied automatically (unless you accidentally copy it to the "beginning" of the reversed string -- this is obviously a logic error). You have to add it manually to do it correctly.
Yea, as soon as I saved that edit, I came to the same conclusion.
NULL should not be used as a char (see stackoverflow.com/questions/2599207/…); use 0 or '\0'.
|
3

The sizeof operator (it is not a function!) is evaluated at compile time. You are passing it a pointer to a region of memory that you claim holds a string. However, the length of this string isn't fixed at compile time. sizeof(str)/sizeof(char) will always yield the size of a pointer on your architecture, probably 8 or 4.

What you want is to use strlen to determine the length of your string.

Alternatively, a more idiomatic way of doing this would be to use std::string (if you insist of reversing the string yourself)

std::string reverse(std::string str) {
  for (std::string::size_type i = 0, j = str.size(); i+1 < j--; ++i) {
    char const swap = str[i];
    str[i] = str[j];
    str[j] = swap;
  }
  return str;
}

Note that due to implicit conversion (see overload (5)), you can also call this function with your plain C-style char pointer.

Improve this answer

Comments

2

There are two issues here:

  1. The sizeof operator won't give you the length of the string. Rather, it gives you the size of a char* on the machine you are using. You can use strlen() instead to get the

  2. A c-string is terminated by a NULL character (which is why strlen() can return the correct length of the string). You need to make sure you are not accidentally copying the NULL character from your source string to the beginning of your destination string. Also, you need to add a NULL character at the end of your destination string or you will get some unexpected output.

Improve this answer

Comments

0 
#include <bits/stdc++.h>

using namespace std;

vector<string> split_string(string);

// Complete the reverseArray function below.
vector<int> reverseArray(vector<int> a) {
    return {a.rbegin(), a.rend()};

}

int main()
{
    ofstream fout(getenv("OUTPUT_PATH"));

    int arr_count;
    cin >> arr_count;
    cin.ignore(numeric_limits<streamsize>::max(), '\n');

    string arr_temp_temp;
    getline(cin, arr_temp_temp);

    vector<string> arr_temp = split_string(arr_temp_temp);

    vector<int> arr(arr_count);

    for (int i = 0; i < arr_count; i++) {
        int arr_item = stoi(arr_temp[i]);

        arr[i] = arr_item;
    }

    vector<int> res = reverseArray(arr);

    for (int i = 0; i < res.size(); i++) {
        fout << res[i];

        if (i != res.size() - 1) {
            fout << " ";
        }
    }

    fout << "\n";

    fout.close();

    return 0;
}

vector<string> split_string(string input_string) {
    string::iterator new_end = unique(input_string.begin(), input_string.end(), [] (const char &x, const char &y) {
        return x == y and x == ' ';
    });

    input_string.erase(new_end, input_string.end());

    while (input_string[input_string.length() - 1] == ' ') {
        input_string.pop_back();
    }

    vector<string> splits;
    char delimiter = ' ';

    size_t i = 0;
    size_t pos = input_string.find(delimiter);

    while (pos != string::npos) {
        splits.push_back(input_string.substr(i, pos - i));

        i = pos + 1;
        pos = input_string.find(delimiter, i);
    }

    splits.push_back(input_string.substr(i, min(pos, input_string.length()) - i + 1));

    return splits;
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.