By Najam Ul Saqib


2018-09-14 15:07:19 8 Comments

Kindly check my code of reversing the array, I can't find the error. It is showing the same array before and after the function call which I don't know why?

#include <iostream>
using namespace std;

void reverseArray(int *arr, int size)
{
    int *Copy_arr = new int[size];
    int temp;
    for (int i = 0; i < size; i++)
    {
        Copy_arr[i] = arr[i];
    }

    for (int i = 0; i < size; i++)
    {
        temp = arr[i];
        arr[i] = arr[(size - 1) - i];
        arr[(size - 1) - i] = temp;
    }

    cout << "Array before function call was: ";
    for (int i = 0; i < size; i++)
    {
        cout << Copy_arr[i] << " ";
    }
    cout << endl << "Array after function call is: ";
    for (int i = 0; i < size; i++)
    {
        cout << arr[i] << " ";
    }
    cout << endl;
}

void main()
{
    int size;
    cout << "Enter size of array\n";
    cin >> size;
    int *p = new int[size];
    cout << "Enter the elements of array\n";
    for (int i = 0; i < size; i++)
    {
        cin >> p[i];
    }
    reverseArray(p, size);
}

3 comments

@Yves Daoust 2018-09-14 15:32:33

You can use the following solution to reverse an array in C++:

for (int i = 0, j= size - 1; i < j; i++, j--)
{
    swap   = arr[i];
    arr[i] = arr[j];
    arr[j] = swap;
}

@Yves Daoust 2018-09-14 15:33:06

Why the downvote ?

@Jean-Baptiste Yunès 2018-09-14 15:35:05

May be lack of explanation? (I'm not the downvoter)

@Jean-Baptiste Yunès 2018-09-14 15:55:46

Worst? No explanation on why OP's code is wrong, and then why yours is correct => downvote

@Abdullrahman Salim 2018-09-14 15:34:23

the problem is that you are not using a buffer to reverse with, so they way it is now is that half of of the reversing loop does what you want and the other half sets everything back. in a way it reverses the reversed array.

as @Jarod42 and @kiran Biradar pointed out that there are better ways of doing this using functions from the standard library, but I assume you want to doing it without the library.

here is a simple fix, replace arr with Copy_arr

for (int i = 0; i < size; i++)
{
    temp = Copy_arr[i];
    arr[i] = Copy_arr[(size - 1) - i];
    arr[(size - 1) - i] = temp;
}

@user463035818 2018-09-14 15:43:13

you "fixed" the problem, but you still do twice as much as needed

@Abdullrahman Salim 2018-09-14 15:47:26

How so? in terms of performance?, he made the Copy_arr anyway so it isn't really that big of a deal

@Jarod42 2018-09-14 15:48:49

for (int i = 0; i != size; ++i) { arr[i] = Copy_arr[size - 1 - i]; } would be enough.

@user463035818 2018-09-14 15:49:19

if you dont use Copy_arr then half of the loop is sufficient, i'd say this is a clear opportunity for saving some work

@Abdullrahman Salim 2018-09-14 15:49:58

I see, but I wanted him to see the error in HIS code so he can understand it and not give him completely new code

@user463035818 2018-09-14 15:51:31

yeah well not saying that this is not an answer, just that it isnt the best way to do it. Look at Jarods answer, he is using a minor modification of OPs code (even less different from OPs than yours ;)

@Abdullrahman Salim 2018-09-14 15:53:24

Well I changed one variable twice in the whole code (so like 2 words), I wouldn't say I'm that different from OP. but you are right.

@user463035818 2018-09-14 15:54:09

btw "the problem is that you are not using a buffer to reverse with" is something to be really critized, doing something in-place is usually prefered if possible and not "a problem" as you state

@Abdullrahman Salim 2018-09-14 15:58:58

Forgive me for not understanding, but you mean like the way I tried to explain it, or like using the word "problem"?

@user463035818 2018-09-14 16:20:27

yes probably just the wording. I would agree with "The problem is caused by ....", sorry for nitpicking ;)

@Jarod42 2018-09-14 15:09:03

in

for (int i = 0; i < size; i++)
{
    temp = arr[i];
    arr[i] = arr[(size - 1) - i];
    arr[(size - 1) - i] = temp;
}

You reverse the array twice.

arr[0] and arr[size - 1] are swapped for i == 0 AND for i == size - 1.

It should be

for (int i = 0; i < size / 2; i++)
{
    std::swap(arr[i], arr[(size - 1) - i]);
}

Or even simpler

std::reverse(arr, arr + size);

@Slava 2018-09-14 15:23:49

Whole function should be std::reverse( arr, arr + size ); Anyway you should point to multiple memory leaks in the OP program.

@Jarod42 2018-09-14 15:30:12

@Slava: For unrelated errors, I tend to use comments for that, and those comments are already present.

Related Questions

Sponsored Content

10 Answered Questions

25 Answered Questions

[SOLVED] Easiest way to convert int to string in C++

11 Answered Questions

[SOLVED] How can I profile C++ code running on Linux?

  • 2008-12-17 20:29:24
  • Gabriel Isenberg
  • 424072 View
  • 1517 Score
  • 11 Answer
  • Tags:   c++ unix profiling

22 Answered Questions

[SOLVED] Why do we need virtual functions in C++?

21 Answered Questions

[SOLVED] What is the "-->" operator in C++?

13 Answered Questions

[SOLVED] What is the effect of extern "C" in C++?

1 Answered Questions

[SOLVED] The Definitive C++ Book Guide and List

  • 2008-12-23 05:23:56
  • grepsedawk
  • 1955694 View
  • 4252 Score
  • 1 Answer
  • Tags:   c++ c++-faq

33 Answered Questions

21 Answered Questions

Sponsored Content