Re: Suggestion for fetchInto

["Tomas V.V.Cox" <cox@idecnet.com>] > 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. So maybe we should change the default to PEAR_ERROR_TRIGGER or PEAR_ERROR_DIE then? > > > - 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. Maybe not postgres, but there are other databases that are able to fail in magnificent ways. Out of memory, row locking timeouts, connection problems. Even if most users don't care, DB should not cut off those who do. > > 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. Errors within the error handling system are the only special case. If your car breaks down, you can't tow yourself to the garage. :-) > > 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)) { > .... > } Well, I'm very sceptical to changing the API, especially when it breaks consistency, which I think not returning errors in fetchRow and fetchInto does. If you insist on having the "dumb and simple" behaviour, it's better to do it with two new methods. - Stig -- Stig Sæther Bakken <ssb@fast.no> Fast Search & Transfer ASA, Trondheim, Norway