Optimisation of code

[moodydog]

I am currently learning C++ following "Programming Principles and Practice" (I absolutely love the book so far) and have a bit of a query about some code I've written.

The aim of the code (as described by the exercise in the textbook, is to get the user to input two integers, and then output the smallest, the largest, the sum, the difference, the product and the ratio (assuming by simple division, don't really care to display as a 1:1 type ratio, a fraction will do).

It's a very simple code and I've implemented it in two ways and both work fine. But I am curious to know, which one of the two ways is better optimised, or better practice for when writing much more complex programs. I know for such a small programs, the difference is probably negligible, but I just wanted to know if there ever is a preference with these sorts of things.

Type 1:

Introduce the two integers, and write a set of if statements conditioned to each value being larger than the other. Then writing a set of actions with respect to the larger value for each condition.

int main()
{
	cout << "Please enter two integer values: ";
	int val1, val2, val3=0;
	cin >> val1 >> val2;	

	if (val1 > val2)	
		cout << "\nSmaller Value: " << val2
		<< "\nLarger Value: " << val1
		<< "\nSum: " << val2 + val1	
		<< "\nDifference: " << val1 - val2
		<< "\nProduct: " << val2*val1
		<< "\nRatio: " << val1/val2 << "\n\n";

	if (val2 > val1)	
		cout << "\nSmaller Value: " << val1
		<< "\nLarger Value: " << val2
		<< "\nSum: " << val2 + val1	
		<< "\nDifference: " << val2 - val1
		<< "\nProduct: " << val2*val1
		<< "\nRatio: " << val2/val1 << "\n\n";
}

Possible Benefits: Doesn't introduce a new variable.
Possible Drawbacks: Larger amount of code, a lot of repetition for each if statement.

-----------------------------------------------------------------------------

Type 2:

Introduce the two integers, and rearrange so the second value is always the largest by introducing a third variable.

int main()
{
	cout << "Please enter two integer values: ";
	int val1, val2, val3=0;					
	cin >> val1 >> val2;

	if (val1 > val2)
		val3 = val2;
		val2 = val1;
		val1 = val3;

	cout << "\nSmaller Value: " << val1
	<< "\nLarger Value: " << val2
	<< "\nSum: " << val2 + val1
	<< "\nDifference: " << val2 - val1
	<< "\nProduct: " << val2*val1
	<< "\nRatio: " << val2/val1 << "\n\n";
}

Possible Benefits: Much shorter code, only one set of instructions.
Possible Drawbacks: Introduces an additional variable.

-------------------------------------------------------------------------

The main question is...

Which one of these methods is preferred from an efficiency and simplicity point of view if both can achieve the answer? (Though to be honest, I won't be surprised if you turned round and said that one method is efficient, and the other is simplistic)

Is it better to use repetition through IF statements and use less variables, or have shorter code and use more variables to express functions (assuming having more variables takes up more memory??). Imagine if my problem was a lot more convoluted, (which in truth would probably require both). I've yet to learn about proper programming techniques, and the effect of adding extra variables to a program, so apologies if my question seems a bit ignorant (but hence why I am asking).

Thanks.

[jlb]

The first snippet, IMO, has too much code duplication and doesn't account for cases where val1 is equal to val2, you would be better off using an if()/else clause instead of two if() statements. IMO, all the printing should be done after if() statements, not in the if statements. Adding extra variables is really of no consequence in such a small program. IMO, the code duplication is a bigger problem than adding "extra" variables.

Also because you failed to use braces for your if() statements the statements only contain one line each.

#include <iostream>

using namespace std;

int main()
{
	cout << "Please enter two integer values: ";
	int val1, val2;
	cin >> val1 >> val2;

	int larger, smaller, sum, difference, product;
	double ratio;

	if(val1 > val2)  // Notice the use of the braces!
       {
           smaller = val2;
           larger = val1;
       }
       else
       {
           smaller = val1;
           larger = val2;
       }

       sum = val1 + val2;
       difference = val1 - val2;
       product = val1 * val2;
       ratio = static_cast<double>(val1) / val2;  // Force floating point division with the cast.

       cout << "\nSmaller Value: " << smaller
		<< "\nLarger Value: " << larger
		<< "\nSum: " << sum
		<< "\nDifference: " << difference
		<< "\nProduct: " << product
		<< "\nRatio: " << ratio << "\n\n";

       return 0;
}

The second snippet is incorrect because your if() statement doesn't have braces, your ratio will also be incorrect because there are no fractions with integers. And IMO swapping the values could be confusing, especially if the program was larger. And because you swapped the numerator and denominator in your "ratio" you might not have the proper "ratio" for what one would expect.

Also be careful about swapping the variables in your if() statements, this can confuse things because most people are used to seeing the terms in the same order.

// Can be confusing IMO, especially in a more complicated program.
	if (val1 > val2)	
...
	if (val2 > val1)	
// I'd prefer to swap the comparison operators.
	if (val1 > val2)	
...
	if (val1 < val2)	

[AbstractionAnon]

I prefer solution #2. When you have repetitive code as in solution 1, it becomes a maintenance nightmare as you have to makes changes in every occurrence. A third solution is to use a function.

void Display (int val1, int val2)
{   cout << "\nSmaller Value: " << val1
	<< "\nLarger Value: " << val2
	<< "\nSum: " << val2 + val1
	<< "\nDifference: " << val2 - val1
	<< "\nProduct: " << val2*val1
	<< "\nRatio: " << val2/val1 << "\n\n";
}
	
int main()
{   cout << "Please enter two integer values: ";
	int val1, val2;					
	cin >> val1 >> val2;

	if (val1 < val2)
	    Display (val1, val2);
    else
        Display (val2, val1); 
    return 0;
}

BTW, lines 8-10 in your second snippet need {} around the statements. Otherwise only line 8 will be conditional.

[Nico]

It will depend on the programs you are going to write (creating and deleting an extra instance of some object may be very expensive in some scenario), but for this example you are doing the exact thing twice.
In that case, if you work as in your first example your programs will become a maintenance nightmare really fast.

The second approach is already a lot more maintainable. Still, I think you should mainly become more aware of build-in functions, for example in this case: std::min and std::max. They would allow you to do the second approach without the extra variable.

http://www.cplusplus.com/reference/algorithm/min/
http://www.cplusplus.com/reference/algorithm/max/

[moodydog]

Thank you for your replies,

Mainly because the book only handles single if statements so far, I wrongly interpreted that as "If statements do not need braces in C++". I must've have only tested one condition the second way. Anyway, I have corrected this. So in general, needless repetition is bad, adding more variables is fine, if you can call some other function to do the sorting work (abstraction?) that is optimal?

(I has actually dawned on me that if I had 4 or 5 values that I needed to sort, the first method would get pretty monstrous, (if (val 1 < val 2), if (val 1 < val3), if (val1 < val4), if (val2 <val3), if (val2 < val4), if (val3 < val4) then output val 1 is the smallest, val 2 is the next smallest...
if (val1 < val2), if (val1 > val3) ... ) I would need some sort of sorting function. )

[jlb]

I would need some sort of sorting function.

Or better yet use a vector to hold the individual values, instead of all the discrete variables, and a sorting function (or just use std::sort).

[Nico]

I would need some sort of sorting function.

If you would like to implement that yourself, I would recommend you to google for "bubble sort", just because it is a understandable and in my opinion fun way to sort a series of values.

If you just want the result there are the build in functions again: http://www.cplusplus.com/reference/algorithm/sort/