Finding the largest number exercise

[Violet_82]

Hi there,
I have done a small exercise (ex 4.17 on C++ how to program - deitel and deitel) which basically is about writing a program that uses a while statement to determine and print the largest number of 10 numbers input by the user. The program should use 3 variables, counter, number and largest.

So, here is the program:

header file:

//exercise_4-17.h file, holding the class definition
class LargestNumber
{
public: 
	void processNumbers( ) ; //processes the numbers
};

cpp file:

//exercise_4-17.cpp file, holding the functions definitions
#include <iostream>
using namespace std ;

#include "exercise_4-17.h"

void LargestNumber::processNumbers( )
{
int counter = 1 ;
int largest = 0 ;
int number ;

while( counter <= 10 )
	
	{
	
		cout << "Insert number: " ;

		cin >> number ;

		if( number > largest )
			
			{
				largest = number ;

				cout << "The largest number so far is: " <<  largest << endl ;
			
			}
		
		else 
		{
			cout << "The largest number so far is: " <<  largest << endl ;
		}
	counter++ ;

	}

}

and main file:

//exercise_4-17_main.cpp holds the main file
#include "exercise_4-17.h"

int main( )
{
LargestNumber numbers; //create LargestNumber object

numbers.processNumbers();

return 0;
}

Now, this exercise was rather easy, so I was wondering whether it would be good practice to like use private members in the class: say, I could declare the variable in the class and make them private and then access them via a getter and change them with a setter. WOuld that have any advantage, or is it an unnecessary complication?
thanks

[PhysicsExpert]

In this case its utterly unneccessary do use classes in the first place. But beside that yes, its considered good practice.

[arkoenig]

I'll reinforce what PhysicsExpert said: I see no reason to define a class here.

That said, I'd like to point out some other problems with your code.

Perhaps the most important problem is that what your program actually does is different from what you said you were asked to do. You said that the program is supposed to read 10 numbers and print the largest. What it actually does, however, is to print 10 numbers, each of which is the largest number encountered so far--unless all of the numbers encountered so far are negative, in which case it prints zero.

Next, you should be consistent in your indentation. For example, whenever you have curly braces, you should indent the text within the braces by the same amount.

You didn't do that here. For example, in your cpp file, line 8 is an open brace, but lines 9 through 11 are not indented. Similarly, line 34 has the same indentation as line 36, which contains a close brace.

When you write an if statement with an else clause, the parts before and after the else should be indented the same. In your code, lines 23-28 are indented more than lines 31-33.

Finally, you have written a fair amount of unnecessary code. In particular, it is possible to reduce lines 21-33 to three lines without much effort. I'll leave it to you to figure out how.

[WaltP]

Hi there,

Hi.

I have done a small exercise (ex 4.17 on C++ how to program - deitel and deitel) which basically is about writing a program that uses a while statement to determine and print the largest number of 10 numbers input by the user. The program should use 3 variables, counter, number and largest.
...
Now, this exercise was rather easy, so I was wondering whether it would be good practice to like use private members in the class: say, I could declare the variable in the class and make them private and then access them via a getter and change them with a setter. WOuld that have any advantage, or is it an unnecessary complication?
thanks

If you don't put any variables in the class, why bother with the class at all? I would do it as a good exercise.

Now let's consider your logic:
You call 1 function which does *all* the work. So why not just leave the code in main() and forget the function?

To make this program more sensible, break it up into it's components. A component should do only one job:
1) Initialize data - set all values to their initial states
2) Input data - accept data from the user
3) Process data - process the data input
4) Output results - display the answer

Each can be a method/function of the class. So your logic in main() becomes

initialize data
loop
    get a value
    process the value
until no more values
display result

Comments on your code:
1) Too much spacing. Set your IDE to 4 spaces when a TAB is hit. It lines up better.
2) Don't indent before *and* after your {. And indent for *every* {.
3) Only 1 line between statements except in strategic places.
4) Don't do exactly the same thing in an IF and the ELSE (your output)
5) Generally, start your counters at 0, not 1. 0 means nothing counted yet. 1 means there has been something counted.

My version of your code:

#include <iostream>
using namespace std ;

#include "exercise_4-17.h"

void LargestNumber::processNumbers( )
{
    int counter = 1 ;
    int largest = 0 ;
    int number ;

    while (counter <= 10)
    {
        cout << "Insert number: ";
        cin >> number ;

        if (number > largest)
        {
            largest = number ;
        }
        cout << "The largest number so far is: " <<  largest << endl;
        counter++ ;
    }
}

[Violet_82]

hi there,
thanks for your comments.

Perhaps the most important problem is that what your program actually does is different from what you said you were asked to do...[

I thought to print and test the number while I was inputting them and I didn't really attempt any validation. Sorry but I am still at the beginning, so try to get to grips with things :)

WaltP, thanks I will bear in mind the function and class issues.