How can you hope to fix (or even recognize) what is wrong if you
don’t know what it will look like when it’s right?
How can you even test it in the first place?
1.1 Read the Documentation
Read all available documentation before you start
testing
Then read it again when you start debugging
Most common advice on any technical help site:
RTFM
1.2 Know What’s Reasonable
Engineers often talk about “sanity checks” and “back-of-the-envelope”
estimates. These refer to quick considerations of what is actually a
reasonable result in a given situation.
So if you see something that isn’t reasonable, your first responses
should be to mistrust the calculations (or, in our case, the code)
that produced that “obviously” unreasonable behavior.
Where do we get the knowledge of what is and isn’t reasonable?
Technical knowledge:
what are common values in various data types?
how does this algorithm normally behave?
Domain & application knowledge:
What will correct output look like?
What are reasonable ranges for values?
1.3 Know the Roadmap
Large programs are made of many components. When you see an output
that is incorrect, your debugging will be much more efficient if you
have an idea which of those components contributed to that particular
output.
What are the pieces of the system?
How do they connect/interact with one another?
Example: if you were debugging this program after seeing an identical
mistake in both reports, would you look first at the report generators
or would you concentrate on the “shared” input processing and main
calculation?
Of course it’s possible that some buggy code was copied-and-pasted
from one report generator to the other, but I’d play the odds and look at the
shared components first.
2. Make it fail …
Typically you start debugging because your program failed
maybe during your own testing
or maybe someone else told you about it.
What’s the next thing you need to do?
Make It Fail Again
Once You’ve Made it Fail …
Make sure you can make it fail again
This is where testing and debugging meet
Why “Make It Fail Again”?
You need to observe the failure
Second hand reports and even your own memories won’t
be detailed enough
You need to figure out what’s causing it
Knowing the conditions under which it fails is an
important clue
You need to know if you have fixed it
If you know how to cause the failure, then apply a
fix, you can see if the failure really goes away.
2.1 Make it fail Reliably
Try to repeat whatever you did to cause the failure
Write down your steps
Try to get to the point where you can automatically
produce the failure.
For programs with textual input, if the
failure is replicatable, you should be able to do this.
E.g.,
program < inputData.txt
This is a good reason not to enter your tests
manually in the first place.
Sometimes, replicating the failure is hard …
Intermittent Failures
A failure that, given the same inputs, occurs only some of
the time is called intermittent
These are the bane of technicians and engineers in
many fields
An intermittent failure may occur 99% of the time when the
buggy input is presented, or only 1% of the time, or even less.
Can waste a lot of time during debugging waiting for
a failure
Can Hardware Failures Be Truly Intermittent?
Sometimes we just don’t see the common factor that causes the failure
So we just “think” the problem is intermittent.
But hardware really is subject to “random” factors such as
vibration, electronic noise, loose connections, temperature
variations, timing variations
People working with hardware need to be creative to
control or reproduce these factors.
Can Software Failures Be Truly Intermittent?
Sometimes
software that controls hardware can have timing
issues
So can software that runs on distributed
hardware
But usually we are simply not seeing the critical
factor that, when repeated, guarantees the failure
Tracking Down Intermittent Failures
Work backwards — the problem may be in an earlier input
sequence
Run repeatedly (invoke the buggy code from within a loop)
add code to detect the failure automatically
Take advantage of the nature of the failure
Focus on the most likely causes of intermittent
failures…
What Causes Intermittent Software Failures?
Typically,
Uninitialized variables
Out of bounds data access
Pointer errors
Uninitialized variables
The errors resulting from this are more likely to be seen in longer test
sequences
Often easy to catch “by eye”
Look for bizzare, apparently random numbers in the output
Manifests more often in Linux than in Windows?
Windows tends to zero out newly allocated memory
Because zero is the most common thing we would have wanted to
use to initialize, this “hides” the problem.
Until the most embarrassing moment possible
Out of bounds data access
What happens if you access arrays using negative or overly large index
values?
May retrieve data that’s actually not in the array, but part of
some other variable
If you store data via the array, you may actually be overwriting
other variables
Timing and actual symptom of failure may depend on how/when those
other variables get used.
Pointer errors - dangling pointers
Dangling pointers
We saw earlier that if bc is destroyed, …
we are left with dangling pointers.
Pointer errors - Deleting a pointer twice
Question: What happens if all the collections were
destroyed and they each deleted the pointer?
Answer
When the first collection is destroyed, its destructor deletes the array.
When the second and third collections are destroyed, their destructors
delete their dangling pointers to the same address.
Deleting the same address multiple times will typically corrupt the
data structures that the run-time system’s memory manager uses to
respond to future requests for memory via new.
So we wind up with a really nasty bug. It probably will not crash the
program now. Instead it will crash it at some point in the future when
our program requests new memory and then uses that.
We might see a failure when trying to allocate the memory.
We might see future new allocations returning the same or
overlapping blocks of memory, causing almost unpredictable results.
It’s so hard to predict what the nature of the failure will be or when
it will occur, that this is almost certain to be one of the hardest
possible problems to debug.
2.2 Make it fail Easily
Debugging often requires you to run the failing test cases over and
over again.
If it takes hours to demonstrate a failure, how long will it
take you to debug it?
Even if it merely takes several minutes, how often are you
going to run that failing code?
Simplify Your Failing Tests
Debugging is primarily an activity of working backwards
from the point of failure towards the cause.
The shorter the path from the start of execution to
the failure, the less you need to look at.
Simplify, Simplify, Simplify
“The only reasonable numbers in a programming
language design are zero, one and infinity.” – Bruce MacLennan (1987)
There are very few things that programs a small, finite number of
times. There are things that they never do, things that they do once
(e.g., initialization and termination code), and things that they do
in loops.
But loops typically have the characteristic that one run through the
loop body is pretty much like any other.
If you made the program fail on the 100th input or 100th
time around a loop, can you find a simpler test that fails after
the 10th time?
Or maybe the 2nd?
Shortening that loop gives you less to debug.
KISS
“Two is an impossible number, and can’t exist.” – I. Asimov (1972)
(Asimov was actually talking about physics, where we sometimes see
zero instances of a phenomenon (things that turn out ot be
impossible), and sometimes see near-infinite numbers of things (stars,
atoms, sub-atomic particles), and occasionally see one unique instance
of a thing (the universe), but the moment you have seen 2 instances of
a thing, the smart money is on the chances of its being much more
common than that.)
Later we will make a distinction between unit
testing and system testing,
Unit tests are typically much simpler
which is one of the major reasons to use them
If It Takes Too Long to Fail…
Redefine failure!
By adding more and earlier output, you can move up the
point of failure and see things going wrong sooner.
We’ve talked about the idea of adding debugging output in an
earlier lesson.
Example: When Do Pointer Errors Fail?
When memory is deleted, it usually gets added
to a “free list”
If we try to access a freed block via a dangling pointer
If the block has not yet been reallocated for a new purpose,
everything still looks OK
Possibly much later in the program, when that block is re-used,
we suddenly start seeing problems.
Example: When Do Pointer Errors Fail? (cont.)
If we delete the same block twice …
Tends to corrupt the free list
Probably won’t fail immediately
Later in the execution, may suddenly be impossible to allocate new
memory
Or may eventually allocate same block twice for two
different variables
Tracking Pointer Errors - Redefining Failure
That time delay between the problem and the actual failure can be hard to deal with
If we can reduce the delay, error will no longer appear
intermittent
Possibilities:
Print addresses being allocated and deleted
Add debugging output to constructors and destructors
Specialized tools (e.g., Purify, LeakTracer)
Idea is to make the failure obvious and early
Related to the next rule, …
2.3 Make it fail Visibly
If a tree falls in the forest and no one is there to hear,
does it make a sound?
If a program fails and no one observes the error, can you
debug it?
Program States
A program state is the combination of
what you’ve got (the values of all program
variables)
where you are (the location to which execution has
reached)
how you got here (the activation stack recording the
places from which all still-active function calls were
made)
Infected States
A program state is infected if one of
its components is affected by a bug :
a bad value in one or more variables, or
at a wrong location (because we followed an incorrect
branch at some conditional statement)
Propagating an Infection
As we move forward through the code, an infected state can
propagate - as bad values in some variables are used to
compute the values of other variables, or
vanish - if the infected values are never written to
output and/or are overwritten by good values
From Infection to Failure
A failure occurs when an infected state affects (propagates to) the program
output.
Debugging is a process of tracing backwards from bad
output towards the cause (faulty code)
Requires viewing of the infected states
Instrument the System
Add debugging output
Tells you where you are
Displays the value of variables suspected of being
infected
Usually written to the standard error stream (cerr)
Minimizes interference with “real” output written to
standard out (cout)
Can be separated out at the command line.
E.g.,
myProgram
writes cout and cerr to screen
myProgram > output.dat`
writes cout to file output.dat but
cerr to the screen
myProgram.exe >& output.dat
(Unix only) writes both cout and cerr to
file output.dat
Design Instrumentation In
Good rule of thumb for C++ ADT designers:
Always provide an output function for your ADT, showing the
value of each data member.
Even if the application itself does not need it
More often than not, you will want it during debugging
And you don’t want to be distracted by designing,
writing and debugging new output functions when
you are already debugging something else.
Build Instrumentation In Later
Actual outputs are likely to be added during debugging
cerr << "I suspect " << x << " is wrong" << endl;
Special macros useful during debugging:
__FILE__ expands to the name of the file
being compiled
__LINE__ expands to the line number in
which it appears
cerr << "I suspect " << x
<< " is wrong at "
<< __FILE__ << ":" << __LINE__<< endl;
During an early run, the seller of a Chinese Vase complained that
a bid for $27 was accepted as a winner even though a reserve price of
$50 had been set.
A check of the items file:
24
10.00 12:00:00 Star Wars Collectables
1.00 23:00:00 Bonds autograph
⋮
50.00 20:06:27 Chinese Vase
⋮
100.00 23:30:00 Cornflake shaped like Illinois
showed that the reserve price was indeed set.
The bids file did indeed contain a bid for the indicated
amount.
void resolveAuction (const Item& item,
BidderCollection& bidders,
BidCollection& bids)
{
double highestBidSoFar = 0.0;
string winningBidderSoFar;
bool reservePriceMet = false;
for (int bidNum = 0; bidNum < bids.getSize(); ++bidNum)
{
Bid bid = bids.get(bidNum);
if (bid.getTimePlacedAt().noLaterThan(item.getAuctionEndTime()))
{
if (bid.getItem() == item.getName()
&& bid.getAmount() > highestBidSoFar
)
{
int bidderNum = bidders.findBidder(bid.getBidder());
Bidder bidder = bidders.get(bidderNum);
// Can this bidder afford it?
if (bid.getAmount() <= bidder.getBalance())
{
highestBidSoFar = bid.getAmount();
winningBidderSoFar = bid.getBidder();
cerr << "Best bid so far is ["
<< bid.getBidder() << " "
<< bid.getAmount() << " "
<< bid.getItem() << " ";
bid.getTimePlacedAt().print(cerr);
cerr << "] from ["
<< bidder.getName() << " "
<< bidder.getBalance() << "]"
<< endl;
}
}
if (bid.getAmount() > item.getReservedPrice())
reservePriceMet = true;
}
}
// If highestBidSoFar is non-zero, we have a winner
if (reservePriceMet && highestBidSoFar > 0.0)
{
int bidderNum = bidders.findBidder(winningBidderSoFar);
cout << item.getName()
<< " won by " << winningBidderSoFar
<< " for " << highestBidSoFar << endl;
Bidder& bidder = bidders.get(bidderNum);
bidder.setBalance (bidder.getBalance() - highestBidSoFar);
}
else
{
cout << item.getName()
<< " reserve not met"
<< endl;
}
}
gives us the output:
Best bid so far is [jjones 26 Hummel Figurine 00:18:03] from [jjones 75]
Hummel Figurine won by jjones for 26
Best bid so far is [ssmith 27 Chinese Vase 04:03:05] from [ssmith 55]
Chinese Vase won by ssmith for 27
all of which looks OK.
Actually I would never write debugging output as clumsy as
that
Compare the effort required to print all the fields of a bid
or bidder to the effort required to print all the fields of a
time
That’s because, for Time, we observed the
rule of thumb to actually provide an output function for our ADTs
#include <iostream>
#include <fstream>
#include "items.h"
//
// Items up for auction
//
using namespace std;
Item::Item()
: reservedPrice(0.0)
{
}
Item::Item (std::string itemName, double reserve, Time auctionEnd)
: name(itemName), reservedPrice(reserve), auctionEndsAt(auctionEnd)
{
}
/**
* Read one item from the indicated file
*/
void Item::read (istream& in)
{
in >> reservedPrice;
auctionEndsAt.read (in);
// Reading the item name.
char c;
in >> c; // Skips blanks and reads first character of the name
string line;
getline (in, line); // Read the rest of the line
name = string(1,c) + line;
}
// print a bidder
void Item::print (std::ostream& out) const
{
out << "[" << name << " " << reservedPrice << " ";
auctionEndsAt.print (out);
out << "]";
}
#ifndef TIMES_H
#define TIMES_H
#include <iostream>
/**
* Times in this program are represented by three integers: H, M, & S, representing
* the hours, minutes, and seconds, respecitvely.
*/
struct Time {
// Create time objects
Time(); // midnight
Time (int h, int m, int s);
// Access to attributes
int getHours() const;
int getMinutes() const;
int getSeconds() const;
// Calculations with time
void add (Time delta);
Time difference (Time fromTime);
/**
* Read a time (in the format HH:MM:SS) after skipping any
* prior whitepace.
*/
void read (std::istream& in);
/**
* Print a time in the format HH:MM:SS (two digits each)
*/
void print (std::ostream& out) const;
/**
* Compare two times. Return true iff time1 is earlier than or equal to
* time2
*/
bool noLaterThan(const Time& time2);
/**
* Compare two times. Return true iff time1 is equal to
* time2
*
*/
bool equalTo(const Time& time2);
// From here on is hidden
int secondsSinceMidnight;
};
#endif // TIMES_H
#ifndef BIDCOLLECTION_H
#define BIDCOLLECTION_H
#include "bids.h"
#include <iostream>
class BidCollection {
int MaxSize;
int size;
Bid* elements; // array of bids
public:
/**
* Create a collection capable of holding the indicated number of bids
*/
BidCollection (int MaxBids = 1000);
~BidCollection ();
// Access to attributes
int getMaxSize() const {return MaxSize;}
int getSize() const {return size;}
// Access to individual elements
const Bid& get(int index) const {return elements[index];}
// Collection operations
void addInTimeOrder (const Bid& value);
// Adds this bid into a position such that
// all bids are ordered by the time the bid was placed
//Pre: getSize() < getMaxSize()
void remove (int index);
// Remove the bid at the indicated position
//Pre: 0 <= index < getSize()
/**
* Read all bids from the indicated file
*/
void readBids (std::string fileName);
// Print the collection
void print (std::ostream& out) const;
};
#endif
#include "bidcollection.h"
#include "arrayUtils.h"
#include <fstream>
using namespace std;
/**
* Create a collection capable of holding the indicated number of bids
*/
BidCollection::BidCollection (int MaxBids)
: MaxSize(MaxBids), size(0)
{
elements = new Bid [MaxSize];
}
BidCollection::~BidCollection ()
{
delete [] elements;
}
// Collection operations
void BidCollection::addInTimeOrder (const Bid& value)
// Adds this bid into a position such that
// all bids are ordered by the time the bid was placed
//Pre: getSize() < getMaxSize()
{
// Make room for the insertion
int toBeMoved = size - 1;
while (toBeMoved >= 0 &&
value.getTimePlacedAt().noLaterThan(elements[toBeMoved].getTimePlacedAt())) {
elements[toBeMoved+1] = elements[toBeMoved];
--toBeMoved;
}
// Insert the new value
elements[toBeMoved+1] = value;
++size;
}
void BidCollection::remove (int index)
// Remove the bid at the indicated position
//Pre: 0 <= index < getSize()
{
removeElement (elements, size, index);
}
/**
* Read all bids from the indicated file
*/
void BidCollection::readBids (std::string fileName)
{
size = 0;
ifstream in (fileName.c_str());
int nBids;
in >> nBids;
for (int i = 0; i < nBids; ++i)
{
char c;
string bidderName;
double amount;
in >> bidderName >> amount;
Time bidPlacedAt;
bidPlacedAt.read (in);
string word, line;
in >> word; // First word of itemName
getline (in, line); // rest of item name
string itemName = word + line;
addInTimeOrder (Bid (bidderName, amount, itemName, bidPlacedAt));
}
}
// Print the collection
void BidCollection::print (std::ostream& out) const
{
out << size << "/" << MaxSize << "{";
for (int i = 0; i < size; ++i)
{
out << " ";
elements[i].print (out);
out << "\n";
}
out << "}";
}
#ifndef BIDDERCOLLECTION_H
#define BIDDERCOLLECTION_H
#include "bidders.h"
#include <iostream>
//
// Bidders Registered for auction
//
class BidderCollection {
int MaxSize;
int size;
Bidder* elements; // array of items
public:
/**
* Create a collection capable of holding the indicated number of items
*/
BidderCollection (int MaxBidders = 1000);
~BidderCollection ();
// Access to attributes
int getMaxSize() const {return MaxSize;}
int getSize() const {return size;}
// Access to individual elements
Bidder& get(int index) {return elements[index];}
// Collection operations
int add (const Bidder& value);
// Adds this bidder to the collection at an unspecified position.
// Returns the position where added.
//Pre: getSize() < getMaxSize()
void remove (int index);
// Remove the item at the indicated position
//Pre: 0 <= index < getSize()
int findBidder (std::string name) const;
// Returns the position where a bidde mathcing the given
// name can be found, or getSize() if no bidder with that name exists.
/**
* Read all items from the indicated file
*/
void readBidders (std::string fileName);
// Print the collection
void print (std::ostream& out) const;
};
#endif
#include <iostream>
#include "arrayUtils.h"
#include <fstream>
#include "biddercollection.h"
using namespace std;
/**
* Create a collection capable of holding the indicated number of items
*/
BidderCollection::BidderCollection (int MaxBidders)
: MaxSize(MaxBidders), size(0)
{
elements = new Bidder [MaxSize];
}
BidderCollection::~BidderCollection ()
{
delete [] elements;
}
// Collection operations
int BidderCollection::add (const Bidder& value)
// Adds this bidder
//Pre: getSize() < getMaxSize()
{
addToEnd (elements, size, value);
return size - 1;
}
void BidderCollection::remove (int index)
// Remove the bidder at the indicated position
//Pre: 0 <= index < getSize()
{
removeElement (elements, size, index);
}
/**
* Read all bidders from the indicated file
*/
void BidderCollection::readBidders (std::string fileName)
{
size = 0;
ifstream in (fileName.c_str());
int nBidders;
in >> nBidders;
for (int i = 0; i < nBidders && i < MaxSize; ++i)
{
string nme;
double bal;
in >> nme >> bal;
Bidder bidder (nme, bal);;
add (bidder);
}
}
/**
* Find the index of the bidder with the given name. If no such bidder exists,
* return nBidders.
*/
int BidderCollection::findBidder (std::string name) const
{
int found = size;
for (int i = 0; i < size && found == size; ++i)
{
if (name == elements[i].getName())
found = i;
}
return found;
}
// Print the collection
void BidderCollection::print (std::ostream& out) const
{
out << size << "/" << MaxSize << "{";
for (int i = 0; i < size; ++i)
{
out << " ";
elements[i].print (out);
out << "\n";
}
out << "}";
}
#ifndef ITEMCOLLECTION_H
#define ITEMCOLLECTION_H
#include "items.h"
#include <iostream>
class ItemCollection {
int MaxSize;
int size;
Item* elements; // array of items
public:
/**
* Create a collection capable of holding the indicated number of items
*/
ItemCollection (int MaxItems = 1000);
~ItemCollection ();
// Access to attributes
int getMaxSize() const {return MaxSize;}
int getSize() const {return size;}
// Access to individual elements
const Item& get(int index) const {return elements[index];}
// Collection operations
void addInTimeOrder (const Item& value);
// Adds this item into a position such that
// all items are ordered by the time at which the auction for the
// item ends.
//Pre: getSize() < getMaxSize()
void remove (int index);
// Remove the item at the indicated position
//Pre: 0 <= index < getSize()
/**
* Read all items from the indicated file
*/
void readItems (std::string fileName);
// Print the collection
void print (std::ostream& out) const;
};
#endif
#include <iostream>
#include "arrayUtils.h"
#include <fstream>
#include "itemcollection.h"
//
// Items up for auction
//
using namespace std;
/**
* Create a collection capable of holding the indicated number of items
*/
ItemCollection::ItemCollection (int MaxItems)
: MaxSize(MaxItems), size(0)
{
elements = new Item [MaxSize];
}
ItemCollection::~ItemCollection ()
{
delete [] elements;
}
// Collection operations
void ItemCollection::addInTimeOrder (const Item& value)
// Adds this item into a position such that
// all items are ordered by the time the item' auction ends
//Pre: getSize() < getMaxSize()
{
// Make room for the insertion
int toBeMoved = size - 1;
while (toBeMoved >= 0 &&
value.getAuctionEndTime().noLaterThan(elements[toBeMoved].getAuctionEndTime())) {
elements[toBeMoved+1] = elements[toBeMoved];
--toBeMoved;
}
// Insert the new value
elements[toBeMoved+1] = value;
++size;
}
void ItemCollection::remove (int index)
// Remove the item at the indicated position
//Pre: 0 <= index < getSize()
{
removeElement (elements, size, index);
}
/**
* Read all items from the indicated file
*/
void ItemCollection::readItems (std::string fileName)
{
size = 0;
ifstream in (fileName.c_str());
int nItems;
in >> nItems;
for (int i = 0; i < nItems; ++i)
{
Item item;
item.read (in);
addInTimeOrder (item);
}
}
// Print the collection
void ItemCollection::print (std::ostream& out) const
{
out << size << "/" << MaxSize << "{";
for (int i = 0; i < size; ++i)
{
out << " ";
elements[i].print (out);
out << "\n";
}
out << "}";
}
Looking at All Bids
Since the determination of the best bid is not obviously wrong, we
might wonder if all the non-best bids are being handled correctly. In
particular, we recall that there were more than two bids for the two
items.
void resolveAuction (const Item& item,
BidderCollection& bidders,
BidCollection& bids)
{
cerr << "resolving item ";
item.print(cerr);
cerr << endl;
double highestBidSoFar = 0.0;
string winningBidderSoFar;
bool reservePriceMet = false;
for (int bidNum = 0; bidNum < bids.getSize(); ++bidNum)
{
Bid bid = bids.get(bidNum);
cerr << "Looking at bid ";
bid.print (cerr);
cerr << endl;
if (bid.getTimePlacedAt().noLaterThan(item.getAuctionEndTime()))
{
if (bid.getItem() == item.getName()
&& bid.getAmount() > highestBidSoFar
)
{
int bidderNum = bidders.findBidder(bid.getBidder());
Bidder bidder = bidders.get(bidderNum);
cerr << "Bidder is ";
bidder.print (cerr);
cerr << endl;
// Can this bidder afford it?
if (bid.getAmount() <= bidder.getBalance())
{
highestBidSoFar = bid.getAmount();
winningBidderSoFar = bid.getBidder();
cerr << "Best bid so far is ";
bid.print (cerr);
cerr << " from ";
bidder.print (cerr);
cerr << endl;
}
}
if (bid.getAmount() > item.getReservedPrice())
reservePriceMet = true;
}
}
// If highestBidSoFar is non-zero, we have a winner
if (reservePriceMet && highestBidSoFar > 0.0)
{
int bidderNum = bidders.findBidder(winningBidderSoFar);
cout << item.getName()
<< " won by " << winningBidderSoFar
<< " for " << highestBidSoFar << endl;
Bidder& bidder = bidders.get(bidderNum);
bidder.setBalance (bidder.getBalance() - highestBidSoFar);
}
else
{
cout << item.getName()
<< " reserve not met"
<< endl;
}
}
gives output
resolving item [Hummel Figurine 25 15:30:11]
Looking at bid [jjones 26 Hummel Figurine 00:18:03]
Bidder is [jjones 75]
Best bid so far is [jjones 26 Hummel Figurine 00:18:03] from [jjones 75]
Looking at bid [jjones 60 Chinese Vase 04:03:01]
Looking at bid [ssmith 27 Chinese Vase 04:03:05]
Looking at bid [jjones 25 Chinese Vase 05:00:00]
Hummel Figurine won by jjones for 26
resolving item [Chinese Vase 50 20:06:27]
Looking at bid [jjones 26 Hummel Figurine 00:18:03]
Looking at bid [jjones 60 Chinese Vase 04:03:01]
Bidder is [jjones 49]
Looking at bid [ssmith 27 Chinese Vase 04:03:05]
Bidder is [ssmith 55]
Best bid so far is [ssmith 27 Chinese Vase 04:03:05] from [ssmith 55]
Looking at bid [jjones 25 Chinese Vase 05:00:00]
Chinese Vase won by ssmith for 27
It’s still not clear why the low bid by smith is being accepted, but
we can see that all bids are in fact being processed in some
fashion.
Since we are not getting enough info about the reserve, let’s also
monitor when the reservePriceMet flag changes.
void resolveAuction (const Item& item,
BidderCollection& bidders,
BidCollection& bids)
{
cerr << "resolving item ";
item.print(cerr);
cerr << endl;
double highestBidSoFar = 0.0;
string winningBidderSoFar;
bool reservePriceMet = false;
for (int bidNum = 0; bidNum < bids.getSize(); ++bidNum)
{
Bid bid = bids.get(bidNum);
cerr << "Looking at bid ";
bid.print (cerr);
cerr << endl;
if (bid.getTimePlacedAt().noLaterThan(item.getAuctionEndTime()))
{
if (bid.getItem() == item.getName()
&& bid.getAmount() > highestBidSoFar
)
{
int bidderNum = bidders.findBidder(bid.getBidder());
Bidder bidder = bidders.get(bidderNum);
cerr << "Bidder is ";
bidder.print (cerr);
cerr << endl;
// Can this bidder afford it?
if (bid.getAmount() <= bidder.getBalance())
{
highestBidSoFar = bid.getAmount();
winningBidderSoFar = bid.getBidder();
cerr << "Best bid so far is ";
bid.print (cerr);
cerr << " from ";
bidder.print (cerr);
cerr << endl;
}
}
if (bid.getAmount() > item.getReservedPrice())
{
reservePriceMet = true;
cerr << "reserve price met by bid ";
bid.print (cerr);
cerr << endl;
}
}
}
// If highestBidSoFar is non-zero, we have a winner
if (reservePriceMet && highestBidSoFar > 0.0)
{
int bidderNum = bidders.findBidder(winningBidderSoFar);
cout << item.getName()
<< " won by " << winningBidderSoFar
<< " for " << highestBidSoFar << endl;
Bidder& bidder = bidders.get(bidderNum);
bidder.setBalance (bidder.getBalance() - highestBidSoFar);
}
else
{
cout << item.getName()
<< " reserve not met"
<< endl;
}
}
which has output
resolving item [Hummel Figurine 25 15:30:11]
Looking at bid [jjones 26 Hummel Figurine 00:18:03]
Bidder is [jjones 75]
Best bid so far is [jjones 26 Hummel Figurine 00:18:03] from [jjones 75]
reserve price met by bid [jjones 26 Hummel Figurine 00:18:03]
Looking at bid [jjones 60 Chinese Vase 04:03:01]
reserve price met by bid [jjones 60 Chinese Vase 04:03:01]
Looking at bid [ssmith 27 Chinese Vase 04:03:05]
reserve price met by bid [ssmith 27 Chinese Vase 04:03:05]
Looking at bid [jjones 25 Chinese Vase 05:00:00]
Hummel Figurine won by jjones for 26
resolving item [Chinese Vase 50 20:06:27]
Looking at bid [jjones 26 Hummel Figurine 00:18:03]
Looking at bid [jjones 60 Chinese Vase 04:03:01]
Bidder is [jjones 49]
reserve price met by bid [jjones 60 Chinese Vase 04:03:01]
Looking at bid [ssmith 27 Chinese Vase 04:03:05]
Bidder is [ssmith 55]
Best bid so far is [ssmith 27 Chinese Vase 04:03:05] from [ssmith 55]
Looking at bid [jjones 25 Chinese Vase 05:00:00]
Chinese Vase won by ssmith for 27
resolving item [Hummel Figurine 25 15:30:11]
Looking at bid [jjones 26 Hummel Figurine 00:18:03]
Bidder is [jjones 75]
Best bid so far is [jjones 26 Hummel Figurine 00:18:03] from [jjones 75]
reserve price met by bid [jjones 26 Hummel Figurine 00:18:03]
Looking at bid [jjones 60 Chinese Vase 04:03:01]
reserve price met by bid [jjones 60 Chinese Vase 04:03:01]
Looking at bid [ssmith 27 Chinese Vase 04:03:05]
reserve price met by bid [ssmith 27 Chinese Vase 04:03:05] ➊
Looking at bid [jjones 25 Chinese Vase 05:00:00]
Hummel Figurine won by jjones for 26
resolving item [Chinese Vase 50 20:06:27]
Looking at bid [jjones 26 Hummel Figurine 00:18:03]
Looking at bid [jjones 60 Chinese Vase 04:03:01]
Bidder is [jjones 49] ➌
reserve price met by bid [jjones 60 Chinese Vase 04:03:01] ➋
Looking at bid [ssmith 27 Chinese Vase 04:03:05]
Bidder is [ssmith 55]
Best bid so far is [ssmith 27 Chinese Vase 04:03:05] from [ssmith 55]
Looking at bid [jjones 25 Chinese Vase 05:00:00]
Chinese Vase won by ssmith for 27
Now we can see, not just one, but two problems.
➊ The Hummel reserve price is being met by a bid on the
Chinese vase
This did not affect the output because other bids also met
the reserve.
➋ The Chinese vase reserve price is being met by a bid from
someone who does not have enough money (\co3) to pay that bid.
So the condition
if (bid.getAmount() > item.getReservedPrice())
{
reservePriceMet = true;
is far too lax.
Diagnosis Precedes the Cure
At this point, we have diagnosed the problem.
That’s not the same thing as actually fixing it, but it’s
close.
In fact, we mainly need to move that test inside the if
statements above it.
