Suggestion: Favour fail! over Option<T> for error states

[mathijshenquet]

When foreign functions are nearly sure to return proper values dont wrap in an Option monad for invalid data but just fail!.

I think it is OK to fail! instead of Option when there is really an error state. The resulting api is also a lot cleaner.

[jeremyletang]

I will be more in favor of change which make:

fn new_opt() -> Option<T>;
fn new() -> T; // fail internally

I think i would like to keep both constructor, i don't like the idea that a non recoverable fail can come from the library,

[mathijshenquet]

In effect, the fail has allready come from the library, gtk+-3.0 in this case.

Also, i'm not sure how a user might respond to a failed $widget_type::new().

Found some info on g_object_new failure: http://blogs.gnome.org/desrt/2012/02/26/a-gentle-introduction-to-gobject-construction/comment-page-1/#comment-1233

I agree with you though that non recoverable fail's aren't nice and should be prevented as much as possible. It is a hard balance to strike.

[GuillaumeGomez]

I don't agree to fail!. Library shouldn't stop because of an internal error if it's not a critical one.

[mathijshenquet]

When ffi::widget_new returns a null pointer, that is an critical error

[jeremyletang]

Yes, critical error inside gtk, but not for the whole program, maybe the developer want to make some log, or printing some debug on standard output.

[mathijshenquet]

He can just pipe stderr, gtk already spews errors there.

[mathijshenquet]

Btw I like your suggestion as a compomise, adding both like this:

fn new_opt() -> Option<T>;
fn new() -> T; // fail internally

The rust std also fails sometimes, like http://doc.rust-lang.org/std/vec/struct.Vec.html#method.get

[jeremyletang]

Ok, but i want comme back on this, Rust convention give prefix to the case the less predictable or less safe.

Option is the right way to handle error in Rust, maybe we should make it the default choice like this:

fn new() -> Option<T>;
fn new_unchecked() -> T; // fail internally

This is a known pattern in Rust std lib.

[GuillaumeGomez]

It's a good middle, like that it's totally up to the user.

[mathijshenquet]

I strongly believe we should do it reversed, because:

• This is in line with what other libraries are doing (pyGtk, Vala, etc...)
• gtk_new returning a null pointer can only really happen in case of serious failure (out of memory) etc..., i think a hard fail is warranted in that case since you cant really recover in a meaningful way from these errors within the application (a supervisor process should recover).
• The only sensible recovery from a null pointer i can come up with is some general cleaning up and log writing which I have a proposal for below.

So I suggest we it like this:

fn new_opt() -> Option<T>; // Or 
fn new() -> T; // fail internally

And allow the user the register a callback in case they want to do something in case of failure:

fn main(){
    gtk::on_error(|error_code|{
       //send sos, save the woman and children
    })
}

Then, when the user really want the manual null check they can do new_opt anyway.

PS. How do you get github to syntax highlight like that Jeremy :) ?

[jeremyletang]

I maintain that i prefer that Option stay default. See the rust std, they are currently putting all the internally failing function behind the deprecated flag, and removing the _opt suffix to make nomads the default.

I think that as Rust is a really different language we can't make rgtk inline with the others gtk bindings.

Users can too define a macro like the try! std macro which fail on Err() for a Result.

// Ok call fail!() if the return is None, else return the window stored in Some()
let mut window = check!(gtk::Window::new());

But it's finally not really different than call unwrap() on an Option.

To highlight your code just put the Rust flag on your snippet:

[code ...]

[mathijshenquet]

Ok very well,

You can close this issue then. I don't see much point in adding:

fn new_unchecked();

Since it is doesnt save keystrokes vs just calling unwrap()