Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return codes for success and failure (int, bool) #6436

Closed
heinrich5991 opened this issue Mar 17, 2023 · 7 comments
Closed

Return codes for success and failure (int, bool) #6436

heinrich5991 opened this issue Mar 17, 2023 · 7 comments
Labels
meta-discussion Discussion about the project itself, direction, (code) design, etc

Comments

@heinrich5991
Copy link
Member

How do we want to communicate success and failure in the code base? According to #6429 (comment), we're currently using 0 as success and non-0 as failure for functions returning int. We use true as success and false as failure for functions returning bool.

These are currently opposites, and since you can't see (at a call site) whether a function returns int or bool, it seems error-prone to me. I think it'd make sense to unify this somehow. E.g. #6429 moves this towards true for success.

@Robyt3 argues that he finds if(function()) { /* success */ } more readable than if(function()) { /* failure */ }.

For functions returning different kinds of errors, it might make more sense to treat 0 as success because there's usually one success path but many failure paths. I'm not sure if we even use that in the code base. str_hex_decode returns multiple error codes, but I don't think they're used by any caller.

Prior art

curl uses 0 as success. zlib uses 0 as success. WINAPI mostly uses non-0 as success. I think most POSIX APIs that don't return a value use 0 as success.

@heinrich5991 heinrich5991 added the meta-discussion Discussion about the project itself, direction, (code) design, etc label Mar 17, 2023
@def-
Copy link
Member

def- commented Mar 17, 2023

New functions should use bool return types in my opinion.

@heinrich5991
Copy link
Member Author

Should they return true for success or failure?

@def-
Copy link
Member

def- commented Mar 19, 2023

true for success of course.

@Learath2
Copy link
Member

Learath2 commented Apr 5, 2023

bool returns and true for success is what I'd also vote for. If we were sticking with C, I'd probably prefer int returns and 0 for success.

For functions returning different kinds of errors I'm honestly not sure how best to handle them, should they maybe be using enums for their return type? With an enum class we could maybe override the bool cast. if(!function()) { /* handle error */ } is a really pretty pattern so it's quite annoying that we can't have it for those kinds of functions unless we pick 0/false for success.

@Robyt3
Copy link
Member

Robyt3 commented Apr 7, 2023

For enums I think both if(foo() != FOO_SUCCESS) { /* handle error */ } and if(foo() == FOO_SUCCESS) { /* handle success */ } are good patterns.

@edg-l
Copy link
Member

edg-l commented May 14, 2023

true for success

bools to convey the meaning instead of int

@heinrich5991
Copy link
Member Author

true for success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta-discussion Discussion about the project itself, direction, (code) design, etc
Projects
None yet
Development

No branches or pull requests

5 participants