[cairo] cairo_status_to_string and why I don't like CAIRO_OK

[Owen Taylor]

On Thu, 2005-06-09 at 16:54 -0700, Carl Worth wrote:
> On Thu, 09 Jun 2005 10:19:38 -0400, Owen Taylor wrote:
> > I don't think we should write this. If we are frequently writing
> > 
> >  if (status) {
> >  }
> > 
> > Then we should simply write
> > 
> >  if (!status) {
> >  }
> >
> > They are both pretty unreadable until you learn the idiom. 
> 
> If both of these forms appear in the code, then there's no idiom to
> learn. The code is just plain hard to read.

I think there is an idiom "status evaluated as a boolean tells you
whether an error occurred". I don't see a difference between the
positive and negative form for that.

A preference for if (status) rather than if (!status) falls from
a general stylistic thing to avoid negative tests when 

> Yes, 'error' would solve the problem. And I almost proposed exactly
> that earlier. The problem comes when dealing with the common case of
> things actually working. What do we name the success value?
> CAIRO_ERROR_NONE ? Somehow I find it less satisfying to say "this
> function didn't fail" instead of "this function succeeded".

Certainly the avoidance of ERROR_NONE or ERROR_SUCCESS is nice.

[...]

> > > One of the original intentions of CAIRO_OK was to replace uses of "if
> > > (!status)" when testing for success. I agree that that version is
> > > abhorrent and shall not appear in the implementation. I think that "if
> > > (status == CAIRO_STATUS_SUCCESS)" makes a fine replacement for it.
> > 
> > I find that really hard to read and type.
> 
> Why don't we just rewrite that code to use "if (status)" and early
> return on error?
> 
> I just checked, and currently there are only five cases of "if (status
> == CAIRO_STATUS_SUCCESS)" in the code. They all look like this:
> 
> 	status = perform_some_operation ();
> 	if (status == CAIRO_STATUS_SUCCESS)
> 	    follow_up_on_operation ();
> 
> 	return status;

You missed 18 cases of if (STATUS_OK ()) :-). At least some of these
will require gotos to reverse. I'm generally pretty OK with gotos
for error handling, however, though CODING_STYLE recommends against
them.

Or you could just write status == CAIRO_STATUS_SUCCESS for those. 
It's just not worth worrying about ... let's get rid of STATUS_OK
and move on.

Regards,
						Owen

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050610/012b789c/attachment.pgp