Selection control structure style question.

[dacheeba]

Hi everyone. I just have a quick question regarding a conflict of style. Of the two functions list below, which one is considered proper style?

int get_package_weight(){

	int pounds;	// The number of pounds of the package to be delivered,
				// as entered by the user.

	cout << "Enter the weight of the package in pounds: ";
	cin >> pounds;
	// Default range validation so that if the integer variable "pounds" is
		// less than or equivalent to the integer constant
		// "MAX_WEIGHT", then allow pounds to remain as its current
		// value, otherwise the default is to assign "pounds" to be the
		// maximum value accepted for delivery.
	if (pounds <= MAX_WEIGHT)
		;
	else
		pounds = MAX_WEIGHT;
	return pounds;
	}

or

int get_package_weight(){

	int pounds;	// The number of pounds of the package to be delivered,
				// as entered by the user.

	cout << "Enter the weight of the package in pounds: ";
	cin >> pounds;
	// Default range validation so that if the integer variable "pounds" is
		// less than or equivalent to the integer constant
		// "MAX_WEIGHT", then allow pounds to remain as its current
		// value, otherwise the default is to assign "pounds" to be the
		// maximum value accepted for delivery.
	if (pounds > MAX_WEIGHT)
		pounds = MAX_WEIGHT;
	else
		;
	return pounds;
	}

Earlier in the year, my instructor made two statements which are contradictory in this instance. The first thing he said was was that in selection control structures, the true statement should be the condition that occurs the most. The second statement was that selection control structures use an empty statement if there is nothing to do on the default(false statement).

In this case the most likely outcome is that pounds will be less than MAX_WEIGHT, and when that is true it should do nothing.

Which takes precedence as far as good programming style is concerned?

Thanks for any insights!

[JockX]

It's not true that the true condition is the one that happens the most. It is very common to write ifs to cover only borderline scenarios, because main scenario is in the main code, and only special cases should be looket at. What is important style-wise is which condition is clearer when translated to code and which requires action. It is stupid idea to even bother to produce empty else block. My vote is for variation on the second, where you ommit else entirely.

[keskiverto]

// write
if ( MAX_WEIGHT < pounds )
  {
    pounds = MAX_WEIGHT;
  }

// instead of
if ( MAX_WEIGHT < pounds )
  {
    pounds = MAX_WEIGHT;
  }
else
  {
  }

Now, you have three options:

// A
if ( pounds <= MAX_WEIGHT )
  {
  }
else
  {
    pounds = MAX_WEIGHT;
  }
return pounds;

// B
if ( MAX_WEIGHT < pounds )
  {
    pounds = MAX_WEIGHT;
  }
return pounds;

// C
return ( MAX_WEIGHT < pounds ) ? MAX_WEIGHT : pounds;

I would not select A.

the true statement should be the condition that occurs the most

I'm not sure whether this should be applied to your example. The guideline makes much more sense when both branches actually do something.

[dacheeba]

Thanks for the advice. Unfortunately, my instructor requires that we close our if statements with an else, even if it is an empty statement. Probably because its an intro class.

keskiverto, that sounds like the right assessment of the guideline. and option C looks best to me. I always forget about the ternary operators! Thanks!

[JockX]

Unfortunately, my instructor requires that we close our if statements with an else, even if it is an empty statement. Probably because its an intro class.

Well, you probably should be introduced to KISS principle during the course as well :D
http://en.wikipedia.org/wiki/KISS_principle
Just miss that lone semicolon while rearranging the code and comments...

[dhayden]

The purpose of requiring the "else" clause is to make sure you've thought about what happens in that case. I know professional programmers who do this although I don't personally.

As for which one to put first, the goal is to make the code clear. That might mean:
- whichever one makes the condition expresssion simplest.
- whichever one makes the code in the "if" case shortest. I often do this to avoid having an "else" that's separated from its "if" by a great distance.

[dacheeba]

Oddly enough, our instructor has not mentioned KISS at all this semester. I know about it in principle, but in practice, I tend to overthink problems and their solutions. :-P