July 23, 2012

Add a to both sides

Homework 6.2b (class stack with a pointer data member p). The inner parentheses are unnecessary because == has higher precedence than - in C and C++. The outer parentheses are unnecessary because you don’t need parentheses around the return value of a function in C and C++. Put the space outside the parentheses, not inside.

	bool empty() const {
		return( (p - a) == 0 );
	}

	bool empty() const {
		return p - a == 0;
	}

	bool empty() const {
		return p == a;
	}

The & and the [] cancel each other out.

HW 6.2b.

	bool full() const {return p > &a[stack_max_size - 1];}

Get rid of the subtraction.

	bool full() const {return p >= &a[stack_max_size];}

Get rid of the & and [].

	bool full() const {return p >= a + stack_max_size;}

Wrong return value

HW 6.2b. This size returns the number of elements in the data member a, a number that does not change. It was supposed to return the number of int’s currently stored in the stack, a number that changes every time we call push or pop.

	size_t size() const { return ( size_t (sizeof a / sizeof a[ 0 ] ) ) ; };

No reason to convert the expression sizeof a / sizeof a[0] to the data type size_t. This expression is already of the data type size_t. No need for a semicolon after a function definition.

	size_t size() const {return sizeof a / sizeof a[0];}

The above function return the capacity of the stack. The following function returns the number of int’s currently stored in the stack.

	size_t size() const {return p - a;}

No need to assign the same value to p again and again.

HW 6.2b. Do not allow this function to change the value of src. i should be size_t, not int. Write prefix ++ where possible. The parentheses around the subtraction are unnecessary because - has higher precedence than - in C and C++. Get rid of the subtraction: it’s already been written once and for all in the size member function.

//
//
//
stack::stack( stack& src )  {
	for( int i=0; i<stack_max_size;i++ )  {
		a[i] = src.a[i];
		p = a + (src.p - src.a);
	}
}

I deleted the comments.

stack::stack(const stack& src)
{
	for (size_t i = 0; i < src.size(); ++i) {
		a[i] = src.a[i];
		p = a + src.size();
	}
}

stack::stack(const stack& src)
{
	for (size_t i = 0; i < src.size(); ++i) {
		a[i] = src.a[i];
	}

	p = a + src.size();
}

Copy constructor always constructs an empty object

HW 6.2b.

stack::stack(const stack& another)
{
	for (size_t i = 0; i < stack_max_size;++i) {
		a[i] = another.a[i];
	}

	p = a;	//means p = &a[0];
}

stack::stack(const stack& another)
{
	for (size_t i = 0; i < another.size(); ++i) {
		a[i] = another.a[i];
	}

	p = a + another.size();
}

The real way to write the copy constructor in HW 6.2b

The above for loop has already been written for us. It’s in the algorithm (i.e., function) copy.

//in stack.C
#include <algorithm>	//for the copy algorithm

stack::stack(const stack& another)
{
	p = copy(another.a, another.a + another.size(), a);
}

The copy constructor is now short enough to be inline.

//in stack.h
#include <algorithm>

	stack(const stack& another) {p = copy(another.a, another.a + another.size(), a);}

Behaves unpredictably

HW 6.2b. The parens around the subtraction are unnecessary, because - has higher precedence than << in C and C++. The subtraction is unnecessary because it has already been written once and for all in the size member function.

The bug. The value of p is the address of the next free element in the array. The value of the expression *p is the value of the next free element in the array. But this element contains an unpredictable value, so the if behaves unpredictably.

stack::~stack()	//destructor
{
	//cout << "destructor for stack\n";

	if (*p != 0){
		cerr << "Warning: stack still contains " << (p - a) << " value(s).\n";
	}

stack::~stack()	//destructor
{
	//cout << "destructor for stack\n";

	if (p != a){
		cerr << "Warning: stack still contains " << p - a << " value(s).\n";
	}

stack::~stack()	//destructor
{
	//cout << "destructor for stack\n";

	if (!empty()){
		cerr << "Warning: stack still contains " << size() << " value(s).\n";
	}

Now the destructors in Homeworks 6.2a and 6.2b are identical.

No reason to convert a size_t to a size_t

HW 6.2a.

	size_t size() const { return ( size_t ( n ) ) ; }

The data member n is already of data type size_t, so there’s no reason to convert it to size_t.

	size_t size() const {return (n);}

Don’t need parentheses around the return value of a function.

	size_t size() const {return n;}

Code that does nothing.

HW 6.2a. The call to empty returns a bool which is ignored an thrown away.

stack::~stack()	//destructor
{
	//cout << "destructor for stack\n";

	empty();
	if (n != 0) {
		cerr << "Warning: stack still contains " << n << " value(s).\n";
	}
}

stack::~stack()	//destructor
{
	//cout << "destructor for stack\n";

	if (!empty()) {
		cerr << "Warning: stack still contains " << size() << " value(s).\n";
	}
}

Code that is never executed

The if is always false, because of the p = a Never write an if that is always true or always false. Never give an operand to p that is not the address of a dynamically allocated block of memory.

stack::~stack()	//destructor
{
	p = a;
	delete [] p;
	if (! empty()) {
		cerr << "Warning: stack still contains " << size() << " values.\n";
	}
}

stack::~stack()	//destructor
{
	if ( !empty()) {
		cerr << "Warning: stack still contains " << size() << " values.\n";
	}
}

Why doesn’t it push i onto the stack?

void stack::push(int i)
{
	if (full()) {	//overflow
		cerr << "Can't push when size " << size() << " == capacity "
			<< stack_max_size << ".\n":
		exit(EXIT_FAILURE);
	}

	p++;
}

//in stack.h
typedef int value_type;

void stack::push(value_type i)
{
	if (full()) {	//overflow
		cerr << "Can't push when size " << size() << " == capacity "
			<< stack_max_size << ".\n":
		exit(EXIT_FAILURE);
	}

	*p++ = i;

	//The above statement can be split into
	//*p = i;
	//++p;
}