July 16, 2012

Unnecesssary loop

Homework 4.2a. year and julian are int variables.

	// count down the years until we hit 0, adding 'julian' as we go
	for ( int temp_year = year; --temp_year > 0; julian += 365 ) {}

	julian += 365 * (year - 1);   //means julian = julian + 365 * (year - 1)

Unnecesssary loops

Homework 4.2b. All variables are int. The code is correct: it loops the correct number of times.

	if ( initial_year > -1 ) {
		// for AD dates, count down the years until we hit 0, adding to day as we go
		for ( day = initial_day - 1; initial_year-- > 0; day += 365 ) {}
	}
	else
	{
		// for BC dates, subtract a year's worth of days for each BC year
		for ( day = initial_day - 1; initial_year++ < 0; day -= 365 ) {}
	}

Traditional to say >= 0 instead of > -1.

	if (initial_year >= 0) {
		day = 365 * initial_year + initial_day - 1;
	} else {
		day = 365 * initial_year + initial_day - 1;
	}

So all you need is

	day = 365 * initial_year + initial_day - 1;

No reason to keep m alive after the loop is over

Homework 4.b.

It’s a waste of time to put the zero into julian, because the zero gets wiped out in the very next line.

int date::julian() const
{
	int m = month;
	int julian = 0;

	for (julian = day; --m > 0; julian += date_length[m]){
	}

	return julian;
}

Error-prone to use the same name for a function and a variable.

int date::julian() const
{
	int j = day;

	for (size_t m = month; --m > 0; j += date_length[m]) {
	}

	return j;
}

Wrong bounds

Homework 4.2b. Class date with one int data member, day.

int date::month_part() const {
	int remainder = day % 365;
	if (remainder < 0) {
		remainder += 365;
	}

	int dd = remainder;
	int mm = 0;
	for( mm=0; dd >= 0; mm++ ) {
		dd -= date_length[mm+1];
	}
	return mm;
}

No reason to keep dd alive after the loop is over. Use prefix increment where possible. No reason to execute the +1 over and over. Put the space outside the parentheses, not inside.

int date::month_part() const {
	int remainder = day % 365;
	if (remainder < 0) {
		remainder += 365;
	}

	int mm = 1;
	for (int dd = remainder; dd > 0; ++mm) {
		dd -= date_length[mm];
	}
	return mm;
}

Get rid of the unnecessary variable.

int date::month_part() const {
	int dd = day % 365;
	if (dd < 0) {
		dd += 365;
	}

	int mm = 1;
	for (; dd > 0; ++mm) {
		dd -= date_length[mm];
	}
	return mm;
}

Bug

Homework 4.2b. If count is 365, the member function should move the date one year forward. Instead, it moves the date one day forward.

int date::next ( int count )
{
	div_t d = div( count , 365);
	day += d.rem;
	if ( d.rem < 0)  {
		day += 365;
		--d.quot;
	}
	++day;
}

Incompletely constructed object

class chill {
	float temp;
	float speed;
	float windchill;
public:
	chill(float initial_temp, float initial_speed);
	void calc();
	void print() const;
};

chill::chill(float initial_temp, float initial_speed)
{
	//error checking omitted for brevity
	temp = initial_temp;
	speed = initial_speed;
}

void chill::calc()
{
	windchill = 35.74 + 0.6215 * temp - 35.75 * pow(speed,0.16) + 0.4275 * temp * pow(speed,0.16);
}

void chill::print() const
{
	cout << "temp: " << temp << "\n"
		<< "speed: " << speed << "\n"
		<< "windchill: " << windchill << "\n"
}

The constructor leaves the windchill data member full of garbage. What would go wrong if you said

	chill c(32, 20);
	c.print();

Remove the calc function and change the constructor to

chill::chill(float initial_temp, float initial_speed)
{
	//error checking omitted for brevity
	temp = initial_temp;
	speed = initial_speed;
	const float p = pow(speed, 0.16);
	windchill = 35.74 + 0.6215 * temp - 35.75 * p + 0.4275 * temp * p;
}