Hi Stuart, Nice piece of job ! I will nonetheless try to be constructive and give you some remarks I made to myself while reading the patch:
* Shouldn't IOFactory protocol have a docstring to describe the possible options ? (So that it is possible to extend the protocol even further, e.g. when working with Eclipse IFile, IResources, etc., and other "file" I/O APIs as well) * The 2d line of the docstring of reader function "reads" "The default implementations of this protocol ...", but reader is not a protocol, it's a function ... * same for writer function * same for input-stream function * same for output-stream function * I have bad feelings when I see IOFactory being half the time half implemented (e.g. for Reader only the reader parts), even if in the end it's the same thing for users * I also have bad feelings when I see the Coercions protocol defined but not used in conjunction with IOFactory for default behaviours * Decidedly, I have bad feelings when I read about the "magic" of "coercing" a String first as a URL, and if not possible, fall back and consider it a local absolute/relative path. I'm "mitigated" in the sense that either it's too magic and should got rid of, either it's interesting and could be promoted as a behaviour in the predefined Coercions ? * If you define IOFactory on Object, you can then (if I understand correctly, I didn't play with protocols that much) avoid all the (assoc default-streams-impl ...) and go with one of the extend-type... macros and just redefine the overriden parts ? Or is it an (premature ?) optimization ? * Could it be interesting to have as-reader as-inputstream as-outputstream as-writer so that the do-copy methods could all be resolved in terms of those primitive coercions, then resulting in less handled combinations in the end of the methods definitions ? * Could as-file be type hinted so that it's not necessary to do the cast to ^File in as-relative-path ? * It's not clear from 'file function that it can take any as-file-able * Couldn't make-parents be renamed create-parents (interestingly it's the verb "create" that is used in the javadoc) * It's not clear from make-parents docstring that it accepts same arguments as 'file function * After reviewing it again, it's not clear what the function 'file does in its various arity forms (it can somehow be guessed from the arg names, but ...) HTH, -- Laurent 2010/5/11 Stuart Halloway <stuart.hallo...@gmail.com>: > Assembla Ticket #311 [1] calls for the promotion of clojure.contrib.io into > clojure (as clojure.java.io). I have attached a patch, and am requesting > comments and code review from the community. > > Reasons you want to take time from your day to read this code: > > (1) It's important. This isn't just a copy/paste from contrib, there are > several design changes (documented in the ticket). > > (2) It's interesting, demonstrating use of Clojure 1.2 protocols. > > (3) If you are new to Clojure, it isn't that scary. It's "just I/O". :-) > > Thanks! > Stu > > [1] > https://www.assembla.com/spaces/clojure/tickets/311-clojure-java-io-namespace > > -- > 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 -- 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
