Re: Suggestion for fetchInto

Stig Sæther Bakken wrote: > > ["Tomas V.V.Cox" <cox@idecnet.com>] > > > > The approach to improve the fetch row user usability could be this: > > > > - DB_extensions always returns DB_errors on errors as they actually do, > > so if you set up a default PEAR error hlandler, errors could be catched > > by it. > > Whether a function that raises an error returns the error object or > not should never vary, that only leads to confusion, obscure bugs and > grinding of teeth. Besides, if you set the default error handler to > for example PEAR_ERROR_PRINT, you will still want the object back. Sure, but we can't expect from people to set an error handler. > > - Change DB_result::fetchInto to only returns "null" or "array". If it > > get an error from the extention, can use trigger_error (as used in > > PEAR::setErrorHandling) to notice it but always returns "null" in this > > case. > > - The DB_OK constant value should be changed from "0" to "1", so > > don't > > disturbs loops like while. > > Tomas, this patch breaks consistency in a way that will confuse users. > If we want to get people used to the idea of returned error objects, > introducing little exceptions here and there will only be disruptive. > > What you're doing in the patch below is basically replacing the PEAR > error mechanism with trigger_error() in _one_ case inside one of > PEAR's classes. If we want to offer an error system, we should stick > to it. For the moment the unupdated inline documentation of DB::fetch* is: * @return array a row of data, or false on error :) Seriously, I think that nobody checks for errors on fetch loops. What kind of error could occur here? The native extensions (like pg_fetch_row) only return data or false. Perhaps I'm wrong but for example in pg_fetch_row the only things can force an error are: invalid result id (already checked by PEAR) or invalid row number (also checked). In both situations, if you call pg_errormessage before pg_fetch_row you'll get an empty string. > Also, keep in mind how many variations you can have on the behaviour > of trigger_error(): track_errors option, error_reporting option, > error_log option, custom error handlers set by the script that are > also outside PEAR's error concept. I said trigger_error because it is being used now by some PEAR fuctions like PEAR::setErrorHandling. > I agree that the current syntax of fetchInto is a little bit verbose, > but I don't think this is the way to deal with it. We're talking > about (strlen("DB_OK === ") = 10) characters more typing here. Isn't only 10 more characters. If you want to be strict: fetchInto ********** while (DB_OK === ($err = $res->fetchInto($row))) { } if (DB::isError($err)) { .... } fetchRow ******** while (is_array($row = $res->fetRow())) { } if (DB::isError($row)) { .... } Tomas V.V.Cox