weavejester / medley

A lightweight library of useful Clojure functions
Eclipse Public License 1.0
870 stars 67 forks source link

Suggested addition: index-of #54

Closed orestis closed 4 weeks ago

orestis commented 3 years ago

The humble indexOf -> you have a known small vector or sequence, you are doing ops on it and you want to find a particular element's index. (Because you have to e.g. replace it later, or put something before or after it).

Naive implementation:

(defn index-of
  "Return the first index of x inside a seq of xs, or nil"
  [xs x]
  (->> xs
       (medley/indexed)
       (medley/find-first (fn [[_idx item]]
                            (= item x)))
       (first)))
borkdude commented 3 years ago

If you already have a vector, you can just use .indexOf in the implementation, for performance.

orestis commented 3 years ago

True, if you know what you have. But .indexOf will also use identical? instead of =, right? This might be a reason to add this more generic index-of -- if someone needs absolute performance they would stick with vectors and use .indexOf. In any case, this is more a convenience (for me) when manipulating very small vectors (e.g. up to 10 items)

borkdude commented 3 years ago

@orestis I don't think it does:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/APersistentVector.java#L186-L191

I mean, if you're going to implement this as part of this library, it's fairly cheap to check vector? first and then call this, and for other cases fall back on the seq behavior.

darkleaf commented 1 month ago

Vectors, Lists, and LazySeqs are implemented java.util.List. List has indexOf.

We use this implementation in our product. @weavejester would you like to accept PR with this code?

(defn index-of
  "Returns the index of the first occurrence of `x` in `xs`."
  [^java.util.List xs x]
  (if (nil? xs)
    -1
    (.indexOf xs x)))
(t/deftest index-of-test
  (t/testing "with `nil` collection"
    (t/is (= -1 (sut/index-of nil :a))))
  (t/testing "with an empty collection"
    (t/is (= -1 (sut/index-of [] :a))))
  (t/testing "with an absent value"
    (t/is (= -1 (sut/index-of [:a] :b))))
  (t/testing "with a present value in a vector"
    (t/is (= 1 (sut/index-of [:a :b :c :d :b] :b))))
  (t/testing "with a present value in a list"
    (t/is (= 1 (sut/index-of '(:a :b :c :d :b) :b))))
  (t/testing "with multiple occurrences"
    (t/is (= 1 (sut/index-of [:a :b :b :c :d :b] :b))))
  (t/testing "with different data types"
    (t/is (= 2 (sut/index-of [2 "one" 3] 3)))
    (t/is (= 1 (sut/index-of ["one" "two" "three"] "two")))))
weavejester commented 1 month ago

That's an interesting approach, and seems a reasonable function to add in. I think the docstring should read:

Returns the index of the first occurrence of x in a sequential collection, coll, or -1 if not found.

But other than that, I think it's a good addition.