Re: Some comments about GVFS



On Thu, 3 May 2007 11:11:39 +0200, "Benjamin Otte" wrote:
>                    I much prefer the cairo model, where basically the
> object keeps its own GError and has a function like cairo_status [3]
> that gives you the last error that occured.

It's worth pointing out an additional aspect of the cairo
error-handling model: The object becomes entirely inert as soon as an
error occurs within it. That is, all methods of the object first check
the status and return immediately on error.

That part of the model is essential for the application code to be
able to benefit by deferring error-checking to a convenient time.

-Carl

PS. After I wrote that much I also wrote the following rambling
treatise on the model. I didn't have time to make it shorter, so feel
free to delete it. (I almost deleted it myself, but I think someone
might find it useful).

I think the model is extremely successful in that it actually becomes
practical to write correct programs. We started with the assumption
that programmers rarely include all the necessary checks to make their
program correct, so we decided to make the program correct without all
of those checks.

And that means that the correct program can remain readable---there's
a lot less clutter without from error-checking paths. I love the fact
that code samples in the documentation of the library don't need those
often-seen disclaimers, "error-checking code removed for readability".

Another great benefit is that compiler features such as
attribute(warn_unused_result) can actually become feasible to use,
(that is, it doesn't result in a bunch of false-positive noise).

For example, in cairo's core API of about 200 functions, there are
less than 15 that can return an error status indication, (not counting
the calls where the user is explicitly asking for a status value). All
other functions are either void or are returning a value that is of
direct interest to the caller, (such as a constructor or a "get"
function). So, from the calling convention, it's obvious to the user
that the great majority of the time there's no further error-checking
required. And, in the few functions that _do_ return an error status,
it's actually very important for the user to check those, (and
something like the warn_unused_result attribute can be quite helpful
for this).

One thing that can be get trickier with this model is tracking down
what actually caused the error once it is detected. While it's
convenient to be able to defer the error checking, this also means
that detection of the error becomes separated in time and code from
when the error was committed. (With the "old" approach there is often
unintended deferral due to missing error-checks, but hopefully the
user gets lucky and a crash happens soon after the error. But the
"inert" stuff described above prevents this.)

So, to successfully adopt this model, the user really needs to be
provided with some means for getting early reports about detected
errors. Cairo is quite conservative about this, providing only a
function that can be set as a breakpoint for when an error is
detected---and that's probably not quite as much help as will be
desired in many cases. Within a glib world there should be no
compunction in spewing messages or allowing applications to register a
callback for when errors are detected by the library.

Also, another subtle issue is that application code can be incorrect
if it depends on side-effects of library calls that will not always
happen in the face of inert methods. For example, imagine some
fictitious code that looks like this:

	while (collection_has_more_items (collection)) {
		...
		collection_remove_item (collection);
	}

	...

	/* Finally check status of collection here. */

	if (collection_has_error (collection))
		handle_error();

If collection_remove_item could become inert, then the implicit
side-effect of reducing the return-value of collection_has_more_items
would be violated, and the application code would result in an
infinite loop.

So this would be a situation where the inert-object style would not
help at all. Writing a correct program in this case would be more
difficult, (the user would have to anticipate the problem and add an
extra call to check for errors within the loop), than if the
remove_item call directly returned an error indication, (letting the
user know it is important to check for that error).

For cairo, we largely get to ignore this issue, simply because the
side-effects of cairo operations, (drawing operations), rarely have
such a direct impact on a program's control flow.

So that's something else to keep in mind if considering this style.

Finally, cairo also avoids returning NULL from failed object-creation
functions. (The idea being to return a non-NULL object that can
indicate what type of failure occurred.) I don't know that that aspect
has been entirely successful. The application code ends up wanting to
check for failed object creation anyway, and not being able to simply
check for NULL makes the application code slightly more awkward.

Even worse, is that the application author might actually check for
NULL which makes the program look like it has correct error-handling
when in fact it does not:

	surface = cairo_surface_create (...);
	/* BUG: This condition will never be satisfied. */
	if (! surface)
		return FAILURE;

So if I had it to do over again, I _might_ reconsider that aspect.

-Carl

Attachment: pgpB9UOwfVooe.pgp
Description: PGP signature



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]