Re: Enhance seq-min and seq-max

[Nicolas Petton]

Michael Heerdegen <address@hidden> writes:

> Michael Heerdegen <address@hidden> writes:

Hi Michael,

>> Like this, maybe?

I like the idea!  In addition to documentation, could you also add
unit tests? (for seq.el, I like when my unit tests test multiple
sequence types).

I left some comments below.

> +  (if predicate
         ^^^^^^^^^
What if `predicate' is nil but `key' is non-nil?
> +      (if key
> +          (let ((first (car sequence)))
                           ^^^
                           You cannot use `car' here
> +            (cdr (seq-reduce
> +                  (lambda (key-x-and-x y)
> +                    (let ((key-y-and-y (cons (funcall key y) y)))
> +                      (if (funcall predicate (car key-y-and-y) (car 
> key-x-and-x))
> +                          key-y-and-y
> +                        key-x-and-x)))
> +                  (cdr sequence)
                      ^^^
                      You cannot use `cdr' either
> +                  (cons (funcall key first) first))))
> +        (seq-reduce
> +         (lambda (x y) (if (funcall predicate y x) y x))
> +         (cdr sequence) (car sequence)))
> +    (apply #'min (seq-into sequence 'list))))
                     ^^^^^^^^^^^^^^^^^^^^^^^
                     I think your version of `seq-min' will break when
                     `predicate' is used for all sequence types but lists,
                     since you don't convert the sequence, but use `car'
                     and `cdr'.

Thanks!
Nico

signature.asc
Description: PGP signature