rewrite-clj

https://github.com/clj-commons/rewrite-clj
lread 2019-06-10T00:05:57.138600Z

@sogaiu yes, I think your point highlights a con of not repeating the test. But a pro is that code is not repeated! :simple_smile: I learned a long time ago that there is no one right way, only approaches and tradeoffs. So yes, I think both approaches have merit.

lread 2019-06-10T00:10:40.143300Z

yes, it took me a bit of time to wrap my head around the custom zipper too! The defn-switchable is really just way to kind of simulate two implementations atop an non existant interface. I think a good doc string on the namespace will help.

lread 2019-06-10T00:12:35.144700Z

back to position check... maybe if/when we add specs to fns this will help?

2019-06-10T11:39:52.151700Z

@lee i agree many things don't have one "right" way, but i don't usually find it's easy to conclude that for the set of options one has considered, there isn't one that is clearly better than the others -- here i think "better" is short for "better for something". as the "something" isn't spelled out, it's hard to say i guess 🙂 at any rate, if we're talking repetition, another option is to have another macro like defn-switchable, but one that just throws if the passed in zipper is not the custom type. i don't typically opt for macros most of the time though... i agree that better docs would help. big advantage is that it probably won't lead to code breaking! regarding specs, unless they are enabled, would they help with checking? i got the sense many folks weren't in to instrumenting for production code -- or may be i misunderstood?

lread 2019-06-10T12:39:18.160400Z

@sogaiu regarding options considered, I really like what Martin has done with cljdoc https://github.com/cljdoc/cljdoc/tree/master/doc/adr - this would surely be overkill for our little discussion on whether or not to double or single check in position fns, but for larger decisions gives a great history for newcomers. regarding specs, yes, I think most folks turn them off for production, but would be there for unit tests.

lread 2019-06-10T15:52:01.162300Z

also with spec on fn, when you read the code, you see more clearly what is expected within the context of the fn. (note to self: does cljdoc include fn specs? I dunno, will have to check)

lread 2019-06-10T15:56:40.164400Z

funny thing @sogaiu, I was checking out zulipchat slack-archives today and noticed we had this same conversation but on a different fn! 🙂 Back in April: > sogaiu: forgot to mention that i tried your updated branch -- it seems to work :slight_smile: i am not sure about the best place to throw from -- should it be position-in-range? or find-last-by-pos -- did you have some reasoning for your current choice? > … > lee: I am throwing from position-in-range to avoid repeating the check in more than one place.