Thanks for the look/review Lukas; fixed up some things in the code that
clears all these points up -- the comments for `join_where` were outdated,
forgot to update it -- renamed some variables and swapped to `array_reduce`
instead of the manual loop. Now that I can see, there's no reason that it
wasn't `array_reduce` before other than some of my personal confusion with
php and it's error messages.
`$delim` is fixed up so the caller just includes spaces if they want like
you thought -- and i do like it more that it's standardized.
I'd like to leave join_where there because I use it in two places and
didn't really want to repeat the logic -- I'm also foreseeing that it can
be used later to create more WHERE statements out of lists.
(Hopefully) final patch being submitted within a few minutes.
On Sat, Jul 4, 2020 at 6:05 AM Lukas Fleischer <lfleisc...@archlinux.org>
wrote:
> On Thu, 02 Jul 2020 at 23:10:40, Kevin Morris wrote:
> > [...]
> > Signed-off-by: Kevin Morris <kevr.gt...@gmail.com>
> > ---
> > doc/rpc.txt | 4 +++
> > web/lib/aurjson.class.php | 66 ++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 62 insertions(+), 8 deletions(-)
>
> Thanks Kevin! A few comments below.
>
> > diff --git a/doc/rpc.txt b/doc/rpc.txt
> > index 3148ebea..b0f5c4e1 100644
> > @@ -472,6 +472,33 @@ class AurJSON {
> > return array('ids' => $id_args, 'names' => $name_args);
> > }
> >
> > + /*
> > + * Prepare a WHERE statement for each keyword: append
> $func($keyword)
> > + * separated by $delim. Each keyword is sanitized as wildcards
> before
> > + * it's passed to $func.
>
> What does that last part of the comment mean? Doesn't sanitization
> happen in $func itself?
>
> > + *
> > + * @param $delim Delimiter to use in the middle of two keywords.
> > + * @param $keywords Array of keywords to prepare.
> > + * @param $func A function that returns a string. This value is
> concatenated.
> > + *
> > + * @return A WHERE condition statement of keywords separated by
> $delim.
> > + */
> > + private function join_where($delim, $keywords, $func) {
> > + // Applied to each item to concatenate our entire
> statement.
> > + $reduce_func = function($carry, $item) use(&$func) {
> > + array_push($carry, $func($item));
> > + return $carry;
> > + };
> > +
> > + // Manual array_reduce with a local lambda.
> > + $acc = array(); // Initial
> > + foreach ($keywords as &$keyword) {
> > + $acc += $reduce_func($acc, $keyword);
>
> I am slightly puzzled by the implementation here; why is there a need to
> have array_push() above and also use the += operator? Can't we simply
> call $func() directly and use array_push() here?
>
> > + }
> > +
> > + return join(" $delim ", $acc);
>
> It might make sense to just use $delim here and use " AND " in the
> caller (I actually skipped this function on my first read and was asking
> myself why the caller doesn't put spaces around "AND", that's the
> behavior people expect from a join function). Either that or rename the
> function slightly; in the latter case it could even make sense to drop
> the parameter altogether and hardcode the AND, unless you think the
> function could be used for something else in the future.
>
--
Kevin Morris
Software Developer
Personal Inquiries: kevr.gt...@gmail.com
Personal Phone: (415) 583-9687
Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery,
Javascript, SQL, Redux
