Member Avatar for Google Spider

Hi, This program is supposed to ask the user how many numbers they want to sum up and then take all the numbers one by one to give the final result. Its working but perhaps you all professional programmers will point out some flaws in it.

#include <iostream>
using namespace std;
int main()
{
	int x,sum=0,count;
	int* ptr;
	cout<<endl<<"How many numbers?"<<endl;
	cin>>x;
	ptr=new int[x];
	for(count=1;count<=x;count++)
	{
		cout<<endl<<"Enter "<<count<<"th number:";
		cin>>*ptr;
		sum+=*ptr;
	}
	cout<<endl<<sum;
	return 0;
}

Thanks for your time.

  • 3 Contributors
  • 5 Replies
  • 127 Views
  • 12 Hours Discussion Span
  • Latest Post Latest Post by Agni

Dani AI

Generated

A compact review and safe rewrite suggestions for 's summing program, plus a few pitfalls not yet fully covered by replies from and .

The original program repeatedly reads into the same memory location, so the array you allocate is effectively unused. If you intend to store values, index the array using a zero-based index range [0 .. n-1]. If you only need the running total, do not allocate an array at all — read each value and add it to an accumulator. Always validate the count and each input read: reject non-numeric input and handle zero or negative counts before allocating memory.

Prefer automatic storage management in modern C++ to avoid manual delete mistakes and pointer-loss bugs. The example below shows a safe, practical approach using std::vector, input validation, and a 64-bit accumulator to reduce overflow risk:

#include <iostream>
#include <vector>
#include <numeric>
#include <limits>

int main() {
    std::size_t n;
    if (!(std::cin >> n) || n == 0) return 1;
    std::vector<long long> vals;
    vals.reserve(n);
    for (std::size_t i = 0; i < n; ++i) {
        long long v;
        while (!(std::cin >> v)) {
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            std::cerr << "Bad input, try again: ";
        }
        vals.push_back(v);
    }
    long long sum = std::accumulate(vals.begin(), vals.end(), 0LL);
    std::cout << sum << '\n';
}

A few extra cautions: if you follow 's idea of incrementing a raw pointer in a loop, keep a copy of the original pointer so you can release it correctly; deleting a moved pointer is undefined. Use tight variable scope and avoid broad using namespace std; as advised. Finally, test edge cases (zero count, negative count, non-numeric input, very large counts and values) and consider whether you actually need to store inputs or can sum them as they arrive.

Recommended Answers

All 5 Replies

Member Avatar for Agni

From your logic you didnt need an array of integers at all. you are writing the value to the same pointer every loop. if you want to use the array do something like this.

cin>>ptr[count]

and then later sum up the array. or else simply use an int var and do something like this

#include <iostream>
using namespace std;
int main()
{
	int x,sum=0,count;
	int ptr;
	cout<<endl<<"How many numbers?"<<endl;
	cin>>x;
	//ptr=new int[x]; not needed
	for(count=1;count<=x;count++)
	{
		cout<<endl<<"Enter "<<count<<"th number:";
                                ptr = 0;
		cin>>ptr;
		sum+=ptr;
	}
	cout<<endl<<sum;
	return 0;
}
Member Avatar for Agni

Also if you decide to use 'new' to allocate memory in your program, dont forget to delete it when its done. For arrays:

delete [] ptr;

Member Avatar for Google Spider

Also if you decide to use 'new' to allocate memory in your program, dont forget to delete it when its done. For arrays:

delete [] ptr;

Oh yes, thanks...you found a bug.

Member Avatar for Nick Evan

The program looks better now. If you want some minor performance tweaks:
change for(count=1;count<=x;count++) to for(int count=1;count<=x;count++) and remove the previous declaration of count.
This has a few advantages:
- the memory declared for count, is cleared after the loop is done
- your program will look better, less declaration at the beginning for vars that are used somewhere in the rest of the program. With a program this small, this isn't a problem, but if you have a big function and you declare a var that you use 100 lines later, it will klose readability
(- the name 'count' will come available for another use)

You could also change: using namespace std; to :

using std::cout;
using std::endl;
using std::cin;

Because you're not using anything else from the namespace.

But if you wish to ignore my advice, that's fine. Your program is good enough for a good grade. ;)

Member Avatar for Agni

And there's a better way to do it if you have to use the dynamic array, you can increment the ptr everytime instead of using the index and then you can sum it also in the same place.

N I think i found quite a few bugs !!!

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.