Just a look over of my current code?

Could someone just look over my code and make any suggestions? I know that my code does exactly what I want and the format is correct. I just am curious if there is anything that I could change to make this easier to read and to streamline. (Nothing complex)

Im still working on the comments because I dont quite understand what they are but ill figure that out. Im currently in my first coding class and I've only taken five weeks so Im very much a beginner. My project description I had to go off is this...

"Create a function that prompts the user to enter a number. The user must be allowed to continue to enter numbers until they enter a negative number. All the numbers must be stored in an array so they can be passed back to the caller. The function should also return the count of the numbers (how many were entered).

Create a second function that computes the sum of the numbers that were entered. The negative number that was entered to finish input should NOT be included in the sum.

Have main print the sum of the numbers as computed by your second function."

Thanks again for all your help guys! I love this site!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
// Author: Jacob Chesley
// Date Created: 2/6/15
// Last Modification Date: 2/6/15
// Lab Name: Arrays
// File Name: Arrays

#include <iostream>

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

/**********************************************************************
*  int ArrayInput(int numbers[])
*
*	Purpose: Holds given numbers until number is a negative.
*
*  	   Entry:  int numbers[]: Stores the numbers that user inputs
*
*  	    Exit:  input: Number of variables (not inputted numbers) 
*			   minus one for the nagative number input
*********************************************************************/

int ArrayInput(int numbers[]) // Input for array
{
	int input; // Variable in array that gives entered valuse a spot in memory
	(input = 0);
	cout << "Please enter your desired values" << endl;
	do
	{
		cin >> numbers[input++];
	} while (numbers[input - 1] >= 0);
	return input - 1;
}

/**********************************************************************
*  int ArraySum(int Addition[], int input)
*
*	Purpose: Adds all numbers that were inputted together
*
*  	   Entry:  int Addition[]: Stores the numbers that user inputs
*
*  	    Exit:  input: Number of variables (not inputted numbers)
*			   minus one for the nagative number input
*********************************************************************/
int ArraySum(int Addition[], int input) // Addition of all entered numbers
{
	int SumOfNumbers = 0; // The added numbers of all given numbers in the array

	for (int idle = 0; idle < input; idle++)
	{
		SumOfNumbers = Addition[idle] + SumOfNumbers;
	}
	return SumOfNumbers;
}
int main()
{
	int data[50]; // Equal to Numbers[] array, used for "pass to"
	int input; // An interger that can be used for multiple values
	input = ArrayInput(data);
	input = ArraySum(data, input);
	cout << "Your number is: " << input << endl;
	return 0;
}
Let me start by saying this code is excellent. It is very well formatted and very clear, and the things I mention below are all extremely minor. Fantastic work.


That said, here are some things I noticed. And again I want to emphasize that these things are very minor and I am really nitpicking here:

1) ArrayInput should probably take a maximum size as a 2nd parameter. As it stands now, if the user inputs more than 50 values, your program will corrupt memory and do 'Very Bad Things'tm

2) The variable names in your ArraySum function are questionable. The name should reflect what the variable represents:

- SumOfNumbers is a fine name. (Personally I would have went with 'sum' because it's just as clear but is less typing. But whatever. SumOfNumbers is great.) The problem is, your array is named 'Addition' ... so why isn't it named 'SumOfAddition'? Or better yet, why isn't your array called 'Numbers'? After all, it is an array of numbers, it's not an array of addition.

- Input is no longer input, but is the size of the array. 'size' would be a much more appropriate name.

- 'idle' is a little weird. It's clearly a loop counter and loop counters usually don't have informative names (typically they're just a 1 letter name like i) so it's not that big of a deal. But why 'idle'? What about it is idle? Just seems strange.

3) Same problem with 'input' in main.
First it is the size of the data, then it's the sum of the data. At no point does that variable contain input -- so calling it input doesn't really make sense.

4) Same problem with 'input' in ArrayInput. 'count' would be a better name since it's a count of how many numbers have been input... and isn't actually the input itself.


5) You can use the += operator on line 52 to shorten it:

1
2
3
4
5
// this:
SumOfNumbers = Addition[idle] + SumOfNumbers;

// is the same as this:
SumOfNumbers += Addition[idle];



6) Line 27 is a little weird. Why in parenthesis? And why not just initialize input when you create it like you do with SumOfNumbers on line 48?
Thank you so much. This is exactly what I was looking for. I have a 72 in class and I'm just trying to make everything perfect to bring up my grade. I remember the teacher actually going over 1, 5, and 6 so this will save me a point or two. I really appreciate you taking the time for typing this while thing up for me.
Topic archived. No new replies allowed.