handle returned result from native methods

[mfkl]

What should we do about results returned by native methods?

Typically int indicating presence of error, such as libvlc_event_attach. Throwing exceptions seems maybe a bit aggressive, but it would be nice to convey the results to the caller and/or maybe log.

[jeremyVignelles]

methods exposed "as-is" should return an int so that it is aligned with the libvlc API. For methods hidden behind a property or an event, I don't see any other alternative than throwing an exception if we want to keep it as ".net-ish" as possible

[mfkl]

Ok, throw it is. For methods returning only 1 or 0, we can use bool I guess.

[jeremyVignelles]

where 1 is success, yes, bool is appropriate. else we need an integer to prepare the eventual future evolutions of this method

[mfkl]

Usually success is 0 in libvlc if I'm not mistaken.

[jeremyVignelles]

yeah, VLC_SUCCESS maps to 0, and VLC_EGENERIC is 1. In libvlc 3.0.1, they might say, "now that method returns 0, 1 and 2, for two different kind of errors, and the code that uses libvlc will not break, because they should compare the return with VLC_SUCCESS, so that's not a breaking change." But we would need to change our return value to reflect that change, so that caller can detect the difference. That's a breaking change.

[jeremyVignelles]

So return value should be an int, and a static class VlcError with const fields Success and GenericError should be created. I'm not a fan of using an enum for that, because the function might return a value not listed.

[mfkl]

So return value should be an int, and a static class VlcError with const fields Success and GenericError should be created.

I think you meant VlcResult instead of VlcError since it has a Success field. But either way, I'm not clear on what you're suggesting. Should we return an int or should we return a custom result type in your opinion?

I think a boolean result is rather user-friendly. That's what libvlcpp uses, too. If libvlc were to introduce a new int result in addition to existing 0 and 1 (which is highly unlikely, I've to say), it'd be a big breaking change anyway (i.e major version), which we would have to handle. They'd probably create another method altogether though.

I like the Result type, as in Rust, but I think it's only worth it if we actually have some (error) information to share. However in most cases with libvlc, we get -1 for errors and no contextual information. So I think bool would be sufficient.

[jeremyVignelles]

Ok, let's use boolean where only 0 and -1 are returned. I was wondering if that would cause incompatibilities in the future if they decided to add another error code.

[mfkl]

Ok, closing for now.