insert->col() and update->col() use variable argument lists #111
Conversation
…ill not show an error if the $value parameter is used.
| @@ -119,12 +119,13 @@ public function orWhere($cond) | |||
| * | |||
| * @param string $col The column name. | |||
| * | |||
| * @return $this | |||
| * @param array $value | |||
There was a problem hiding this comment.
there is extra space, may be you could remove it also for the sake of consistency ?
There was a problem hiding this comment.
i looked at the code and some @params had a blank line between, some did not. What is your preference? I'll fix the others too...
There was a problem hiding this comment.
I think one space is enough for now. I was also looking at the old code.
There was a problem hiding this comment.
still not quite understanding: should @param have one blank line between each declaration, or should they be on consecutive lines with no blank line between them
There was a problem hiding this comment.
There was a problem hiding this comment.
ah. silly me. fixed now (3 places)
| @@ -24,10 +24,11 @@ | |||
| * | |||
| * @param string $col The column name. | |||
| * | |||
| * @return $this | |||
| * @param array $value | |||
| { | ||
| return call_user_func_array(array($this, 'addCol'), func_get_args()); | ||
| return empty($value) ? $this->addCol($col) : $this->addCol($col, $value[0]); |
There was a problem hiding this comment.
I like this may be more performance ?
There was a problem hiding this comment.
don't know. i wanted to make the function signature more explicit so it was easier for IDEs to auto-fill / auto-complete. There is another trickier one in Update.php /~https://github.com/auraphp/Aura.SqlQuery/blob/2.x/src/Common/Update.php#L89 which i will do if this PR gets merged.
Got a few other PRs planned eg a fix for #100 but will start getting merge conflicts if i make too many new PRs before the current ones get merged or rejected...
There was a problem hiding this comment.
Actually you can just use $this->addCol($col, ...$params); . I have tried https://3v4l.org/sJ6S0 . I can merge some of the PR's . The interface changes probably need more review from @pmjones .
There was a problem hiding this comment.
that is much cleaner! thanks!
| { | ||
| return call_user_func_array(array($this, 'addCol'), func_get_args()); | ||
| return empty($value) ? $this->addCol($col) : $this->addCol($col, $value[0]); |
There was a problem hiding this comment.
Actually can't we just use $this->addCol($col, ...$params); here? Tried https://3v4l.org/sJ6S0 .
|
Thank you. |
so IDEs will not show an error if the $value parameter is used.