-
-
Save echosa/9358930 to your computer and use it in GitHub Desktop.
(ns greed.coords | |
(:require [clojure.core.typed :refer [ann]])) | |
(ann get-next-coordinate-northeast | |
[NonEmptySeqable (Option Number) (Option Number) -> (Option (IPersistentList Number))]) | |
(defn get-next-coordinate-northeast [grid x y] | |
(when-not (or (nil? x) (nil? grid) (>= x (- (count (first grid)) 1)) | |
(nil? y) (<= y 0)) | |
(list (+ x 1) (- y 1)))) | |
;; greed.coords> (clojure.core.typed/check-ns) | |
;; Start collecting greed.coords | |
;; Finished collecting greed.coords | |
;; Collected 1 namespaces in 22.57 msecs | |
;; Start checking greed.coords | |
;; Checked greed.coords in 237.739 msecs | |
;; Checked 1 namespaces (approx. 72 lines) in 260.809 msecs | |
;; Type Error (greed/coords.clj:22:54) Polymorphic function clojure.core/first could not be applied to arguments: | |
;; Polymorphic Variables: | |
;; x | |
;; | |
;; Domains: | |
;; (HVec [x Any *]) | |
;; (Option (EmptySeqable x)) | |
;; (NonEmptySeqable x) | |
;; (Option (clojure.lang.Seqable x)) | |
;; | |
;; Arguments: | |
;; clojure.core.typed/NonEmptySeqable | |
;; | |
;; Ranges: | |
;; x | |
;; nil | |
;; x | |
;; (Option x) | |
;; | |
;; in: (clojure.core/first grid) | |
;; in: (clojure.lang.Numbers/gte x (clojure.lang.Numbers/minus (clojure.lang.RT/count (clojure.core/first grid)) 1)) | |
;; | |
;; | |
;; ExceptionInfo Type Checker: Found 1 error clojure.core/ex-info (core.clj:4327) |
Actually, maybe this:
(ann get-next-coordinate-northeast
[(Seq (Seq Any)) (Option Number) (Option Number)
->
(Option (Vector* Number Number))])
Unless the 0x0 grid is bad, this reads easier to me. :-)
Oh, and are you sure about (<= y 0)
? I would guess (<= 0 y)
is what you're aiming at.
The <= is correct because we're in a when-not. I have unit tests for this function that cover that. :-)
I do like your rewrite, though. I might steal that and apply it to all related functions (see below).
As for the annotations, this is an already used function in my existing project here. You've changed my return value form a list to a vector. I believe those are mostly interchangeable, at least for how I'm using them, but I'd like to leave it as is if possible. If not, then I have a bunch of other methods to change so that I don't have one out of many returning a vector while the rest return a list. here's the file
Edit: never mind what was said here before. I was stupid and didn't change the return type. The annotations works fine. I'd still like to not change my return type, though, if possible. Unless there's a really good reason to do so.
Sorry, yes - ignore the <=
comment!
I had a look, and List*
is available, so:
(ann get-next-coordinate-northeast
[(NonEmptySeqable (NonEmptySeqable Any)) (Option Number) (Option Number)
->
(Option (List* Number Number))])
FWIW, I can't resist playing while I've got 10 minutes. I'd approach from this angle:
(defn- validate
[grid [x y :as coords]]
(when (and grid x y
(<= 0 x (dec (count (first grid))))
(<= 0 y (dec (count grid))))
coords))
(def compass {:north [0 -1]
:east [1 0]
:south [0 1]
:northeast [1 -1]
...
})
(defn offset
[coords direction]
(map + coords (compass direction)))
(validate grid (offset [2 5] :east))
(Thoroughly untested..)
Hm. I'll have to investigate this compass idea. Thanks for that. I've been wanting to get a code review, but just haven't asked for one yet. This has been enlightening. Feel free to keep info coming (via any means of communication). :-)
Heterogeneous vectors are much better supported than heterogeneous lists or seqs.
Though it's tempting to rewrite the function as: