Fix Query count error when call to webservice fails #34
Conversation
| if(is_a($this->__resultSet, '\Cake\Datasource\ResultSetInterface')) { | ||
| return $this->__resultSet->total(); | ||
| } else { | ||
| return false; |
There was a problem hiding this comment.
I would think this should return 0 and not false.
There was a problem hiding this comment.
Yes, the count() method is supposed to return integer.
There was a problem hiding this comment.
Returning 0 if something wrong happens during the query execution would let think it really did return 0 items. Is it something that we really want ? If the execute() method can return false, couldn't the count() method do the same ?
There was a problem hiding this comment.
A count method implies a numerical return to me, I'd be confused if I asked for a count, expecting a integer and got a boolean.
The count method should not care if the query executed successfully or not, it's not it's responsibility. Remember the S in SOLID.
There was a problem hiding this comment.
@alaxos When "something wrong happens" an exception should be thrown :)
| if(is_a($this->__resultSet, '\Cake\Datasource\ResultSetInterface')) { | ||
| return $this->__resultSet->total(); | ||
| } else { | ||
| return false; |
There was a problem hiding this comment.
Yes, the count() method is supposed to return integer.
| } | ||
|
|
||
| return $this->__resultSet->total(); | ||
| if(is_a($this->__resultSet, '\Cake\Datasource\ResultSetInterface')) { |
There was a problem hiding this comment.
Space needed after if. Use instanceof instead of is_a().
|
@alaxos Ping |
|
Fixed in #44 |
3 participants
Closes #33