Hi, I don’t know how many are familiar with https://github.com/Softwarepark/exercises/blob/master/transport-tycoon-1.md , I was wondering if anyone could review my solution https://gist.github.com/francesco-losciale/067d31aaef6443107c222babf9bf2675? Thanks

@francesco.losciale correct me if I'm wrong, but this looks a lot like an overly complicated remove https://gist.github.com/francesco-losciale/067d31aaef6443107c222babf9bf2675#file-tt-kata_core-clj-L29

@francesco.losciale

(defn remove-vehicle-original
  [truck vehicles]
  (let [index (.indexOf vehicles truck)
        length (.length vehicles)]
    (into [] (concat (subvec vehicles 0 index) (subvec vehicles (inc index) length)))))

(defn remove-vehicle-idiomatic
  [truck vehicles]
  (into []
        (remove #{truck})
        vehicles))

(ins)user=> (remove-vehicle-original "bar" ["foo" "bar" "baz"])
["foo" "baz"]
(ins)user=> (remove-vehicle-idiomatic "bar" ["foo" "bar" "baz"])
["foo" "baz"]
(cmd)user=>

also, I think you can just use (remove #{truck} vehicles) on its own, since you only need vectors because you were relying on indexing - not sure though

on a more basic level - a function that complex should either be reduced to something simpler or lifted out and put in a defn like I did to test my replacement above

@francesco.losciale I also see that the same anonymous function is copy pasted into multiple parts of the code, I hope I don't have to explain why that's a bad idea

perhaps what you are missing here is that you can pass any number of args to update-in? you don't need to capture the other args lexically in a closure

(ins)user=> (update-in {:a {:b 21}} [:a :b] * 2)
{:a {:b 42}}