-1

It is code to reverse the values as they entered.When I am running the following code. It is taking 8 inputs only. After that it is not printing anything.

#include <iostream>
using namespace std;
int main() {
int n;
cin>>n;
int *p = new int(sizeof(int)*n);
int q = n;
for(int i=0;i<n;i++)
{
    cin>>*p;
    p++;
}
for(int j=0;j<n;j++)
{
    cout<<*p<<" ";
    p--;
}
return 0;
}
VipinSC
  • 11
  • 4
  • What do you mean "it is not printing anything"? Executing it, it is nearly printing your desired output, except the first element is always `0`so it will never print the last: `n=8`, output=`0 8 7 6 5 4 3 2 ` – M.K Apr 01 '19 at 06:03
  • It was not printing values inside of 2nd loop. Now i got the solution. It should be int *p = new int[n]; It should be an array. Thank you – VipinSC Apr 01 '19 at 06:06
  • 2
    `new int(sizeof(int)*n)` allocates a *single* `int` and initializes it to `sizeof(int)*n`. You need to spend more time with [good books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282), find new tutorials, or go through your class-notes again. – Some programmer dude Apr 01 '19 at 06:12

3 Answers3

0

You can also try the following answer.

#include <iostream>
using namespace std;
int main() {
int n;
cin>>n;
int *p = (int *)malloc(sizeof(int)*n);
int q = n;
for(int i=0;i<n;i++)
{
    cin>>*p;
    p++;
}
for(int j=0;j<n;j++)
{

    p--;
    cout<<*p<<" ";
}
free(p);
return 0;
}
Akhilesh Pandey
  • 868
  • 8
  • 18
0
#include <iostream>
using namespace std;

(Not related to the title, but using namespace std is bad practice that can lead to breakage when switching compilers, for example. Better write the std:: prefix when needed, such as std::cin >>.

int main() {
int n;
cin>>n;
int *p = new int(sizeof(int)*n);

The above is allocating a single int object, whose value is sizeof(int)*n, and p points to that integer. You probably mean:

int *p = (int*)malloc(sizeof(int)*n); // bad style
... at the end:
free(p);

But using malloc in C++ is a bad idea, unless you want to go closer to the operating system for educational purposes.

Slightly better is to use new, which besides allocating the objects also calls their constructors (but nothing is constructed for basic types such as int).

int *p = new int[n]; // so-so style
... at the end:
delete [] p;

The above is not the best practice because it requires manual memory management. Instead, it is recommended to use smart pointers or containers whenever possible:

std::vector<int> p(n);
// continue with the code, just like with the pointers

Or allocate the individual elements only when needed.

std::vector<int> p;
p.reserve(n);  // this is a minor optimization in this case
// ...
   if (int value; std::cin >> value)
      // This is how to add elements:
      p.push_back(value); 
   else
      std::cin.clear();

This looks ok:

int q = n;
for(int i=0;i<n;i++)
{
    cin>>*p;
    p++;
}

But this is broken. When the loop starts, p points after the last element. The following *p dereferences a pointer which goes past the last element:

for(int j=0;j<n;j++)
{
    cout<<*p<<" ";
    p--;
}

Replacing the order of pointer decrement and dereference avoids the crash:

for(int j=0;j<n;j++)
{
    p--;
    std::cout << *p << " ";
}
Michael Veksler
  • 8,217
  • 1
  • 20
  • 33
-1

Ok, there many issues here:

int *p = new int(sizeof(int)*n);

This memory allocation is wrong. It allocates n times sizeof(int) bytes, so if int is 4 bytes long it will allocates n * 4 integers.

int q = n;

q variable is never used.

for(int i=0;i<n;i++)
{
    cin>>*p;
    p++;
}

There is no need for pointer arithmetic here. It would be better to access the array in simple p[i] way.

for(int j=0;j<n;j++)
{
    cout<<*p<<" ";
    p--;
}

Same here...

return 0;
}

You allocated memory, but never deallocate. This will cause memory leaks.

A better, correct version of the program could be:

#include <iostream>

using namespace std;

int main()
{
    int n;
    cin >> n;

    int * p = new int[n];
    for (int i = 0; i < n; ++i)
    {
        cin >> p[i];
    }
    for (int i = (n - 1); i >= 0; --i)
    {
        cout << p[i] << ' ';
    }
    delete [] p;
    return 0;
}
Diodacus
  • 663
  • 3
  • 8