Clean up the API for consistency #4
Conversation
rymndhng
force-pushed
the
cleanup-api
branch
from
5c0816a to
40db9b9
Compare
1. Remove boxing & allocation where possible 2. Make API consistent (remove keyword args, because it cannot be mixed with primitive typehints)
rymndhng
force-pushed
the
cleanup-api
branch
from
40db9b9 to
ac8a340
Compare
| (.incrementCounter client metric sample-rate tags) | ||
| (.incrementCounter client metric tags)))) | ||
| ([metric] | ||
| (.incrementCounter client metric -empty-array)) |
There was a problem hiding this comment.
FWIW i split it up this way because I think this would be a faster path on the critical path because it does away w/ adding sample rate or tags (by calling .incrementCounter directly)
| sample-rate ^Double sample-rate] | ||
| (if sample-rate | ||
| (.recordGaugeValue client metric value sample-rate tags) | ||
| (.recordGaugeValue client metric value tags))))) |
There was a problem hiding this comment.
@gzmask I cleaned this API up a bit to remove the need to create an anonymous function
There was a problem hiding this comment.
Reason of the anonymous function is that I wanted to get rid of the propagate effect of type annotations - so that clojure code remains clojurish and Java types are localized to where the interop happens. Not sure if this is a good practice.
There was a problem hiding this comment.
I'm uncertain what propogate effects you're thinking of.
From the outside you won't need to know about the typehints. The typehints are used to help the clojure compiler pick a function rather than use reflection.
| (defonce ^:private ^StatsDClient client (NoOpStatsDClient.)) | ||
|
|
||
| (defn str-array [tags] | ||
| (def ^:private ^"[Ljava.lang.String;" -empty-array (into-array String [])) |
There was a problem hiding this comment.
I was trying to look up the ^"[Ljava.lang.String;" thing and didn't know what to google for 😕
There was a problem hiding this comment.
It's a typehint for a string array
gzmask
left a comment
gzmask
left a comment
There was a problem hiding this comment.
Can you add back deps for test.check?
see:
https://github.com/unbounce/clojure-dogstatsd-client/pull/3/files#diff-3f8f408345fa9ba7415df1b86d9e6424R11
|
This PR hasn't removed test.check. Can we re-base #3 after this issue is merged? |
2 participants
primitive typehints)
Fixes #2