bug#81581: [PATCH 1/1] Add a LESSP argument to 'seq-min' and 'seq-max'

Philip Kaludercic [14/Aug  4:07pm GMT] wrote:
> Sean Whitton <[email protected]> writes:
>
>> Philip Kaludercic [09/Aug  4:29pm GMT] wrote:
>>>> Code looks good, interesting approach with the eval-when-compile
>>>> (I would have used a cl-loop since it doesn't seem like it's truly a
>>>> reduction, but what you have is fine).
>>>
>>> Why isn't this a reduction?
>>
>> Just a defeasible aesthetic intuition.
>
> My counter-argument is that min is a associative operation, and given a
> non-empty list kind-of unital as well, so for me it fits the bill.
>
>>> Sean Whitton <[email protected]> writes:
>>>
>>>> Sean Whitton [09/Aug 11:50am +01] wrote:
>>>>> Code looks good, interesting approach with the eval-when-compile
>>>>> (I would have used a cl-loop since it doesn't seem like it's truly a
>>>>> reduction, but what you have is fine).
>>>>
>>>> Well, a while loop, probably, given seq.el is preloaded.
>>>
>>> Hmm, but that would amount to just manually looping over the sequence
>>> (seq-length seq) many times and comparing the current best element with
>>> that in (seq-elt seq i)?
>>
>> Yes.
>
> I would vote against this, since when reducing over a list this would
> require a O(n) access for every element of the list (O(n) again), while
> seq-reduce, via seq-do calls mapc internally, that iterates over the
> list directly.

That's fine, I was just making a few comments.

Using a keyword argument, though, I do think you should change.

-- 
Sean Whitton