If else if indenting style - Comments welcome

[gatmalice]

Hi there. I'm a mechanical engineering student taking a class called introduction to engineering solving methods using C++. I'm fairly new to C++ but I have a bit of hobbyist experience with scripting languages like lua.

Basically I was wondering if someone would be willing to comment on my commenting style, and my indenting style regarding if else if statements and et cetera.

Any positive criticism is certainly welcome. Thanks!

/*  Write a C++ function with inputs equal to four integers and outputs equal to
    the largest two of the four integers. Also write a main program to prompt
    the user for four integer inputs, call the function, and display the
    outputs. Test the program with the following 4 integers: -4, 32, 1, 8. */

#include <iostream>  // contains cout/cin functions
using namespace std;
//______________________________________________________________________________
// Prototypes:
    void calc(int, int, int, int, int&, int&);
//______________________________________________________________________________
// Main function:    
int main() {
    int val1, val2, val3, val4, largest1, largest2; // values to assess

    cout << "Please enter 4 integers:";
    cin >> val1 >> val2 >> val3 >> val4;

    calc(val1, val2, val3, val4, largest1, largest2); //function to calculate
    
    cout << "The largest integers are " << largest1 << " and " << largest2 << endl;
    
    system("PAUSE");
    return 0;
}
//______________________________________________________________________________
// Function definitions:
void calc(int Val1, int Val2, int Val3, int Val4, int& Largest1, int& Largest2) {
    Largest1 = Val1;        // initially set Largest1 for comparison throughout the function
    if (Val2 > Largest1) {  // Compare Val2 to Largest1
        Largest1 = Val2;
        Largest2 = Val1;
    } else {
        Largest2 = Val2;
    }
    if (Val3 > Largest1) {  // Compare Val3 to Largest1
        Largest2 = Largest1;
        Largest1 = Val3;
    } else if (Val3 > Largest2) {
        Largest2 = Val3;
    }
    if (Val4 > Largest1) {  // Compare Val4 to Largest1
        Largest2 = Largest1;
        Largest1 = Val4;
    } else if (Val4 > Largest2) {
        Largest2 = Val4;
    }
}
//______________________________________________________________________________ 

Additionally, is there a more efficient way to hash through the values, comparing each one to Largest1 & Largest2, as I've done in void calc()? Please do not do post a code snippet for me (it's my homework), but ideas would be appreciated!

[moorecm]

Looks fine to me. I would add a few blank lines to space things out a bit but that's all.

As far as solving the problem, that looks like it'll work. Two other alternatives are to use min() and max() to select the values or to sort the values and take the two largest.

[z05DSL3A]

comment on my commenting style

On the whole it is neat and easy to read.

Some of the comments don't add to the understanding of the code and should be left out,
eg if (Val3 > Largest1) { // Compare Val3 to Largest1
it is obvious that you are comparing val3 to Largest1.

As moorecm said use blank lines to break up sections.

I would also suggest using better names, calc() doesn't give you much idea of what it does, if you give use better names the code becomes even easier to read and self documenting.

[gatmalice]

Thanks guys!

Is this better?

Additionally, is this sort of thing what the beginner's section of the forums for?

/*  Write a C++ function with inputs equal to four integers and outputs equal to
    the max two of the four integers. Also write a main program to prompt
    the user for four integer inputs, call the function, and display the
    outputs. Test the program with the following 4 integers: -4, 32, 1, 8. */

#include <iostream>  // contains cout/cin functions
using namespace std;
//______________________________________________________________________________
// Prototypes:
    
    void calc_maximum(int, int, int, int, int&, int&);
//______________________________________________________________________________
// Main function:
    
int main() {
    int val1, val2, val3, val4, max1, max2; // values to assess

    cout << "Please enter 4 integers:";
    cin >> val1 >> val2 >> val3 >> val4;

    calc_maximum(val1, val2, val3, val4, max1, max2); // function to calculate highest values
    
    cout << "The maximum integers are " << max1 << " and " << max2 << endl;
    
    system("PAUSE");
    return 0;
}
//______________________________________________________________________________
// Function definitions:
    
void calc_maximum(int Val1, int Val2, int Val3, int Val4, int& Max1, int& Max2) {
    
    Max1 = Val1;        // initially set Max1 for comparison throughout the function
    
    if (Val2 > Max1) {  // if Val2 is greater than Max1, move the value of Max1
        Max1 = Val2;    // down to Max2, and replace with the value of Val2
        Max2 = Val1;
    } else {
        Max2 = Val2;
    }
    
    if (Val3 > Max1) {
        Max2 = Max1;
        Max1 = Val3;
    } else if (Val3 > Max2) {
        Max2 = Val3;
    }
    
    if (Val4 > Max1) {
        Max2 = Max1;
        Max1 = Val4;
    } else if (Val4 > Max2) {
        Max2 = Val4;
    }
}
//______________________________________________________________________________ 

[gatmalice]

@moorecm:

Two other alternatives are to use min() and max() to select the values or to sort the values and take the two largest.

Thanks for the suggestion. I didn't quite see the need for it in this application (I think I would still need an if than structure to prioritize which goes where) but I'll definitely keep those functions in mind for the future =)