@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.
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.
back to position check... maybe if/when we add specs to fns this will help?
@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?
@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.
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)
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.