Fork me on GitHub
#rewrite-clj
<
2019-06-10
>
lread00:06:57

@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.

lread00:06:40

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.

lread00:06:35

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

sogaiu11:06:52

@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?

lread12:06:18

@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.

lread15:06:01

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)

lread15:06:40

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.