Hi Laurent,

Thanks for the detailed feedback! Responses inline (and updated in a new patch with #311).
 * 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)
Yes, added.

 * 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
Yes, added

 * 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
The latter is the point. IOFactory is an abstraction that helps the  
implementer, users don't have to worry about it.
 * I also have bad feelings when I see the Coercions protocol defined
but not used in conjunction with IOFactory for default behaviours
There are very few examples of this, not worth making changing IMO.

 * 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 ?
This is good magic. The space of strings that look like file URLs is  
disjoint from those that look like paths, and both are reasonable  
expectations (based on my ad hoc survey of other languages.)
Bad magic would be to fall back and interpret the name is a classpath  
resource. Some earlier versions did this, and I pulled this into the  
separate fn "resource".
 * 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 ?
No. You cannot define part of a protocol at one level, and then get  
the object behavior for missing parts:
 * 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 ?
I actually tried that, and it make things worse.

 * Could as-file be type hinted so that it's not necessary to do the
cast to ^File in as-relative-path ?
This should work but doesn't yet, protocol type hint handling needs to  
flow through.
* It's not clear from 'file function that it can take any as-file- able
Docstring improved.

 * Couldn't make-parents be renamed create-parents (interestingly
it's the verb "create" that is used in the javadoc)
It's the original name from contrib. I am reluctant to change a name  
people have gotten used to unless the other choice is overwhelmingly  
better. I'll listen to the community on this one.
 * It's not clear from make-parents docstring that it accepts same
arguments as 'file function
I like the docstring as-is but will take a better one if given it.

 * 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 ...)
Improved.

Thanks!
Stu

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

Reply via email to