tarantool/tarantool-php

Incorrect Return Value in readme

nekufa opened this issue · 12 comments

nekufa commented

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?

bigbes commented

C API algorithm:

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

@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?

bigbes commented

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

rybakit commented

@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
}
bigbes commented

It's not something, that bothers you before?

rybakit commented

@bigbes See this comment: #44 (comment)

bigbes commented

Can you provide PR that will fix README.md?

rybakit commented

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

bigbes commented

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?

rybakit commented

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

bigbes commented

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?

rybakit commented

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.