tarantool-php icon indicating copy to clipboard operation
tarantool-php copied to clipboard

Incorrect Return Value in readme

Open nekufa opened this issue 8 years ago • 12 comments

Hello, i found that readme is incorrect.

There are a lot of comments like bool return "False and raises Exception on error". It's impossible to return value and raise the exception.

What will happen? False will be returned or exception thrown?

nekufa avatar Oct 10 '16 21:10 nekufa

C API algorithm:

  1. Exception variable set
  2. return from function with RETURN_FALSE tell me - what will happen?

bigbes avatar Oct 11 '16 09:10 bigbes

@bigbes If I get it right, the question is why a method returns a boolean value. What is the point of returning bool true/false if it will always be true on success and an exception on failure?

rybakit avatar Oct 11 '16 11:10 rybakit

@rybakit Is that a problem? Seems like something, that won't question anyone.

bigbes avatar Oct 11 '16 11:10 bigbes

@bigbes Imho, the problem that api gives a wrong impression that a user can just check the result of a method call, like:

if (!$t->ping()) {
   // this block will never be reached
}

rybakit avatar Oct 11 '16 11:10 rybakit

It's not something, that bothers you before?

bigbes avatar Oct 11 '16 12:10 bigbes

@bigbes See this comment: https://github.com/tarantool/tarantool-php/issues/44#issuecomment-113064903

rybakit avatar Oct 11 '16 12:10 rybakit

Can you provide PR that will fix README.md?

bigbes avatar Oct 11 '16 13:10 bigbes

Of course, I can update README.md if you are OK to tweak the implementation.

rybakit avatar Oct 11 '16 14:10 rybakit

What is the point of returning bool true/false if it will always be true on success and an exception on failure?

Implementation is OK, as I've understand, README gives wrong impression. Not?

bigbes avatar Oct 11 '16 14:10 bigbes

Hm, so you propose to modify the documentation to not reflect the real behavior?

rybakit avatar Oct 11 '16 14:10 rybakit

What is the point of returning bool true/false if it will always be true on success and an exception on failure?

You have told, that this is the real behaviour, not?

bigbes avatar Oct 11 '16 15:10 bigbes

I didn't get your question, sorry. My point (although I'm not alone, as I'm not the one who created this issue /cc @nekufa) that mixing return booleans and exceptions is bad and I've already described and gave an example of why I think the api has the flaw. So, to fix that both the implementation and documentation should be updated to always return void and throw an exception in a case of an error. It doesn't have sense to me to only update the documentation and keep the current behavior. There is nothing much I can add to this discussion, but I hope I was clear enough ;)

P.S. I would really appreciate if you can provide me with the links to the docs or libraries which implement this "bool/exceptions" design, I have never seen this before. Maybe this will help me to understand your implementation.

rybakit avatar Oct 11 '16 15:10 rybakit