On 21 April 2010 22:58, Kevin Livingston <kevinlivingston.pub...@gmail.com> wrote: > Thank you for taking the time to provide feedback.
Thank you for the insightful response! I've posted the current state of my code -- which uses clojure.zip and clojure.walk, among other things -- here: http://gist.github.com/374764 > intentionally driving into an exception seems like a very bad idea. > but yes I can define my own equivalent of consp I was just hoping > there was something I was over looking. Right. I've aliased clojure.contrib.core/seqable? to compound? for now... which I hadn't known about earlier. I guess this was bound to be a solved problem. Agreed on the use of exceptions. > That'll trade some heap for stack space, presumably worthwhile. I > haven't had to do much hand optimization of recursion so I'm a little > rusty (the Franz Allegro compiler was fairly amazing in that > regard)... I was hoping there was some even better trick (like the > trampoline) to save me even more. I did write a version of occurs-check -- which I seem to have renamed to occurs? at some point -- which uses the CPS approach (it's included in the Gist, commented out); then I realised that its a natural use case for clojure.zip (the same basic idea, but with a different packaging). See if you like the current version. > that "extra" paren is there too because the body of the clause is an > implicit progn in common lisp. And an implicit begin in Scheme, I know... Plus the Scheme version handles unary clauses and a special kind of ternary clauses (with '=> in the middle) in special ways, which is useful at times. However, the reason I personally prefer to have those extra parens in cond has much more to do with the shape of the code than those other things. :-) > on some issues I agree others I disagree, regarding your code: Thanks again for your remarks. A few points that came to my mind: > regarding removing the helper functions, that's not necessarily ideal, > while bindings may always be a map putting in literals like {} or nil > instead of fail or no-bindings actually obfuscates the code, and makes > it harder to change later if what bindings is needs to fundamentally > change. I'm in two minds w.r.t. the second point above. On the one hand, I'm loathe to pass on using such facilities as the IFn implementation on maps, sets and vectors in an attempt to make easier some fundamental change which might possibly happen in the future (and who knows what else it might involve besides what I might think of now). On the other, I can already see a different data structure being useful for representing bindings (more on that further down), so I'm also disinclined to hand-wave your argument away. However, I do believe that said different data structure could be made to act like a map fairly transparently (hopefully through the protocol facility; or, if some functionality's missing as of yet, reify), so I'd still prefer to use the map-specific niceties and refactor to remove them only when the need actual comes (and it becomes clear what sort of changes are really needed). > in general it's bad style to wrap a cond with another test if > trivially avoidable (at least this is coming from best practice common > lisp style) so for example, in your code line 24, there's no reason to > pull that out of the cond, especially since you are also using the > return value of the when, it's better to be explicit about the test > and what it's resulting return value is -- in this case that the > unification fails. Well, I was trying to treat failure specially, as it were, but I take your point. Changed now. > the if-let's are a little more interesting as it > potentially saves one lookup, but it seems to make the code far more > "disjoint" or at least increase the number of paths. I've included my cond-let macro in the Gist now (and refactored unify-variable to take advantage of it); do you think it's acceptable? > the changing of a cond to a some with the identity function on line 48 > seems like a very odd choice to me. while still correct code I see no > reason why this would be preferred to a cond. Ouch, I don't know what I was thinking. Changed now. > I'm torn about the simplify-bindings -- I think it's a cool function > to have, because if what you are subbing into is large compared to > bindings, this is probably a win, but if you have a lot of bindings > and are only using some, you could spend a lot of time to pack them > down. it'd probably be good to have around if someone knew where they > were in the trade-off. Perhaps using some kind of local memoization would be a good compromise? There was a cool thread on memoization here a short while ago; see Meikel Brandmeyer's summary here: http://kotka.de/blog/2010/03/memoize_done_right.html I took a shot at that with subst-memo. Additionally, this function leaves unbound variables intact, while the other versions use nil instead. There are two other (simpler) versions of subst included in the current gist, of which one is equivalent to my initial version, while the other one resolves variables when the need arises (and doesn't memoize). I've switched to prewalk for all three versions, like you did. As an alternative to a fancy substitution scheme, I was thinking of a different data structure for bindings, as mentioned above... Something like a reversible map. With reify, it should be able to make such a beast behave as a regular IPersistentMap, but also support lookup-of-key-by-val through a protocol, say, and use that inside assoc to perform resolution at binding time. Not sure if that would really be beneficial, and even if it would, I guess that it might not be worth the trouble when memoization should handle things nicely enough. The idea did make me less certain that all one could ever hope to use as a bindings structure is a hash map, though. ;-) All the best, Michał -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en
