|
|
Posted by Peter Fox on 09/03/06 09:33
Following on from Chung Leong's message. . .
>lawrence k wrote:
>> My own feeling, obviously, is that it is better to error check
>> everything, and to write extensive comments everywhere.
>
>I think you're missing a basic point. Error messages and code comments
>are form of human communication. The merit of saying something versus
>not saying something obviously depends on how the listener makes use of
>the information communicated. Aimless verbosity does no one any good.
>
>If we look at the messages in your example:
>
> "In getWeblogEntries, the function getRow failed to return an array
>on the $i iteration."
> "In getWeblogEntries, the query to the database failed."
>
>Say I'm a programmer picking up your project, how would I respond?
>Well, all I know is that there is something wrong in getWeblogEntries.
>So I do a search for getWeblogEntries, open the file and look. Even
>without your messages though I would have done the exactly the same
>thing. The messages emitted by PHP gives me the file path and line
>number, so they actually do a better job.
>
>In programming as in real life, don't open your month unless you have
>something worthwhile to say. A few judicious utterances is better than
>a constant stream of banalities.
>
Aha! Some sense from Chung Leong
* It is correct that programmers go WCPGW [1] like clocks go tick and
cows go moo.
* But it doesn't necessarily follow that the way to deal with this is
what the OP called 'error checking'.
* Distinguish between
- Errors and exceptions (Exceptions are where something doesn't work
as normal. Errors are where an incorrect result/action is obtained.)
- Detection, reporting and handling
For example if (in the UK) I ask for a date then I want it in
day-month-year order. But you might be used to m-d-y and input to my
form like that.
"1-1-2006" is not an error or an exception
"10-1-2006" is an error (10th Jan accepted though you meant 1st
October)
"1-15-2006" is an exception (you meant Jan 15th but my validation says
there isn't a 15th month)
(Not being able to handle "Unknown" or "2006" may be /faulty/ design.)
I'll leave readers to mull over the d,r,h issues with this example....
* Beware of tests that are not watertight, opaque or make a shot at, but
don't pin down the conditions that matter. In the example code given
<quote>
$row = getRow($result);
if (is_array($row)) {
extract($row);
</quote>
What does the function getRow() return? Here I'll assume it is an alias
for mysql_fetch_array() which returns FALSE if there's a problem. So if
the result is to be tested it is good practice to test according to the
signals in the documentation.
if(FALSE===getRow($result)){
It is the result of extract() that matters in the processing. (Also
it's a handy point to put in an assertion-style test along the lines of
if(3==extract($row)){ // headline,date,mainContent
* Beware of complex 'error' handling introducing new problems.
* Don't forget try...catch
So, to conclude: IMHO the original poster should get A for effort but
would be better off with simply reporting a failure at the end of the
routine. I'd do this like:
function ReportWeblogEntries() {
// ------------------------------------------------
// Prints up to last 30 rows of Weblogs table (latest at top)
// Returns actual number of entries or -ve diagnostic for failure
// ------------------------------------------------
$actualLogCount = -1; // default = failure diagnostic
$query = "SELECT * FROM weblogs ORDER BY date DESC";
if($rows=mysql_query($query)){ // fails on error
$actualLogCount = mysql_num_rows($rows);
// trap stupid number of rows inside loop
for ($i=1; $i <= $actualLogCount; $i++) { // note change of
index base
if($i>30){ // ENOUGH! STOP!
$excess = $actualLogCount-($i-1); // how many still left
to print
print("<hr><b>There are $excess more items (not
shown)</b>");
break;
}
if($row = getRow($rows)){ // can never fail!
if(3==extract($row)){ //headline,date,mainContent -> vars
print( "<h1>$headline</h1>
<h7>$date</h7>";
<div
class=\"mainContent\">$mainContent</div>");
}else{
$actualLogCount = -2; // diagnostic for can never
happen
}else{
$actualLogCount = -3; // diagnostic for can never
happen
}
} // end of step through results loop
mysql_free_result($rows);
}
return $actualLogCount; // normally 0 or +ve for no of rows
}
Note from my version that the function returns /diagnostics/ from the
very first line. Since you have to look at the code to understand
exactly what's happening you don't need more than an alert and possibly
a pointer.
Note that my version frees the database resource under 'all' conditions
that matter.
Note that for the purposes of this exercise I've said "Nobody can
possibly deal with more than 30 items at a time" - so showing more would
be a /fault/ in the design.
Note the original function had a misleading name.
So spend less time on fancyfying the bits that you think might go wrong
and more on discovering what the important failure modes are; and
understanding the context in which the information is going to be used.
[1]What could possibly go wrong
--
PETER FOX Not the same since the bookshop idea was shelved
peterfox@eminent.demon.co.uk.not.this.bit.no.html
2 Tees Close, Witham, Essex.
Gravity beer in Essex <http://www.eminent.demon.co.uk>
[Back to original message]
|