-
Notifications
You must be signed in to change notification settings - Fork 87
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
add missing public methods to SelectInterface and related. #112
Conversation
make reset() public and add isDistinct() to round out the interface
* @return bool | ||
* | ||
*/ | ||
public function hasCols(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a problem with this hasCols
, it sounds like to check if there is a column value exists. But this looks like a check if the columns is empty or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. i can see why it was added (so you can check if that part of the query is valid). What about adding hasCol($name_or_alias)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am ok with that :) .
I will leave this to @pmjones for further review. |
should we do UpdateInterface and InsertInterface here? The changes are mostly pretty simple (copying the declaration from the real class to the interface declaration). So they should be easy to review as one PR. What do you think? |
I feel separate may help. The benefit is if something is likely not going to merge can be rejected other PR can get merged. |
*/ | ||
public function hasCol($alias) | ||
{ | ||
return isset($this->cols[$alias]) || array_search($alias, $this->cols) !== false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to do a search? Doesn't the isset itself help us ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see https://github.com/auraphp/Aura.SqlQuery/pull/112/files/9fb886d8955663988cdfb5c371416ccdc7cdbe27#diff-1e1715bbfd6b71bb485e70bb72002475R99. cols() can add a column or an alias. This finds both, similar to how removeCol() works.
this is (hopefully) the last of the changes needed to fill out the *Interface classes. |
make reset() public and add isDistinct() to round out the interface.