Re: Some comments about GVFS

["Benjamin Otte" <otte gnome org>]

On 5/3/07, Carl Worth <cworth redhat com> wrote:
> So that looks to me like if you modified _cairo_error to do the
> equivalent of g_assert, you'd basically get what you want for your
> recoverable and unavoidable cases. But that would leave you with
> always having g_assert and never g_return_if_fail for programming
> errors though. So please explain more to me about how that handling
> should be distinguished.
I've learned that the difference between a g_assert and a
g_return_if_fail is whose fault it is. g_assert would be used for
internal consistency checks inside cairo, where the cairo developers
ensure they don't write bad code. The best example here is a case
statement as in get/set_property functions, where it's common to add
an assert in the default case.
g_return_if_fail is for developers using your library. It's used to
tell them that they are doing something wrong. It's basically the same
thing, just catering to a different audience.
In short: applications should have no call to g_return_if_fail.
Everything should be g_assert.

There's some advantages to this differentiation:
1) It allows you as a developer to disable internal assertions for
various reasons (try running DBUS with all checks enabled and compare
the speed) while still getting informed about faults in using the
library.
2) It allows adding warnings later on. If people have been using your
library wrong because they didn't know about something, you can add a
g_return_if_fail to that function and the next time they'll run the
program, they'll notice the warning and hopefully fix the bug. GThread
has recently done something like this with printing a warning when
g_thread_init() wasn't the first function called.


There's some things that I don't like about the Glib/Gnome approach:
1) Assertions aren't disabled in releases. This leads to people often
not putting enough assertions where it matters. Noone wants inner
loops to be slow. DBUS does that differently and I think it has really
helped developing DBUS.
2) g_return_if_fail returning often isn't useful and will lead to a
crash. An example here is all those functions that have a GError
argument and return FALSE on failure. They return FALSE in the
return_if_fail case, too, but don't set the error. This of course
causes a program to assume an error was set to crash nonetheless. It
would probably be nice to just return TRUE.


In general, I think being as annoying as possible when a developer
does something wrong is a good thing, because people are lazy. So
unless you force them to care about something, they won't and just
hope it works anyway. So I could probably live fine with making every
g_return_if_fail an assertion. But maybe I'm a bit extreme that way.
I'm known to annoy people with -Werror. ;)


Benjamin