Reading... from a reader

111 views
Skip to first unread message

Phil Hagelberg

unread,
Feb 12, 2009, 12:53:38 PM2/12/09
to clo...@googlegroups.com

I've got a problem where I have a reader (it's a java.io.BufferedReader
that came from duck-streams, if that matters at all) and I want to call
"read" on it.

I'd expect to be able to do (read (reader my-file)), but this isn't OK
because read needs a java.io.PushbackReader. It seems vaguely wrong
somehow to have a function called "reader" return something that cannot
be "read" from.

I took a look at what could be done to fix this. I looked at the clj
definition of read, but it looks like you'd have to turn it into a
multimethod to make it accept readers that aren't PushbackReaders. Since
read is such a fundamental function, this seems ill-advised. But it
looks to me that you could also solve this problem by adding a method to
clojure.lang.LispReader that could accept other kinds of readers and
wrap them in a PushbackReader without modifying any existing code.

Does this seem like a decent idea? I'm fine if the correct answer is
just "quit your whining, it's not that bad", but I'd like to know if I'm
the only person this bothers--at least in that case I can attempt to
adjust my aesthetics. =)

-Phil

Rich Hickey

unread,
Feb 12, 2009, 1:01:26 PM2/12/09
to clo...@googlegroups.com

On Feb 12, 2009, at 12:53 PM, Phil Hagelberg wrote:

>
>
> I've got a problem where I have a reader (it's a
> java.io.BufferedReader
> that came from duck-streams, if that matters at all) and I want to
> call
> "read" on it.
>
> I'd expect to be able to do (read (reader my-file)), but this isn't OK
> because read needs a java.io.PushbackReader. It seems vaguely wrong
> somehow to have a function called "reader" return something that
> cannot
> be "read" from.
>
> I took a look at what could be done to fix this. I looked at the clj
> definition of read, but it looks like you'd have to turn it into a
> multimethod to make it accept readers that aren't PushbackReaders.
> Since
> read is such a fundamental function, this seems ill-advised. But it
> looks to me that you could also solve this problem by adding a
> method to
> clojure.lang.LispReader that could accept other kinds of readers and
> wrap them in a PushbackReader without modifying any existing code.
>
> Does this seem like a decent idea?

Yes. Issue/patch welcome.

Rich

Phil Hagelberg

unread,
Feb 12, 2009, 1:09:22 PM2/12/09
to clo...@googlegroups.com
Phil Hagelberg <ph...@hagelb.org> writes:

> I've got a problem where I have a reader (it's a java.io.BufferedReader
> that came from duck-streams, if that matters at all) and I want to call
> "read" on it.

I should mention that I know *how* to do this:

(read (java.io.PushbackReader. (reader file)))

I just think there could be a better way.

-Phil

Mark Fredrickson

unread,
Feb 12, 2009, 1:31:03 PM2/12/09
to clo...@googlegroups.com
While (read) taking an argument seems valid, I think you can do what
you want in the short term using binding:

(binding [*in* my-reader]
(print (read)))

It has been my general observation that vars + binding in Clojure fill
the niche that optional arguments fill in other languages. While it
may seem like more work to create a binding around a function than to
pass an optional argument, you only have to do it once for many calls
to read. Plus, if you are calling functions that in-turn call read,
your binding will persist. This may or may not be the behavior you
want, but it is certainly powerful. This style of programming tends to
decrease the amount of "tramp data" passed around. I for one am really
enjoying it. I never thought I would be such a fan of dynamic scope.
;-)

IIRC, duck-streams has a (with-reader ...) form that will handle the
binding and close the reader when it exists (which may or may not be
what you want).

Cheers,
_Mark

Stuart Sierra

unread,
Feb 12, 2009, 1:51:55 PM2/12/09
to Clojure
On Feb 12, 12:53 pm, Phil Hagelberg <p...@hagelb.org> wrote:
> I'd expect to be able to do (read (reader my-file)), but this isn't OK
> because read needs a java.io.PushbackReader. It seems vaguely wrong
> somehow to have a function called "reader" return something that cannot
> be "read" from.

I could change "duck-streams/reader" to return a PushbackReader
instead of a BufferedReader. Unfortunately, "readLine" is defined in
BufferedReader but NOT in PushbackReader. We need "readLine" for
"clojure.core/line-seq". So we can't have it both ways. Blame Java's
over-engineered I/O.

Another alternative is to provide a "duck-streams/read*" to wrap
"clojure.core/read" and handle the type conversions.

-Stuart Sierra

Phil Hagelberg

unread,
Feb 12, 2009, 1:59:47 PM2/12/09
to clo...@googlegroups.com
Rich Hickey <richh...@gmail.com> writes:

> Yes. Issue/patch welcome.

Great; issue and patch attachment are here:

http://code.google.com/p/clojure/issues/detail?id=79&colspec=ID%20Type%20Status%20Priority%20Reporter%20Owner%20Summary

I haven't written much Java, but the implementation seems pretty
straightforward. I've submitted my CA already.

-Phil

Phil Hagelberg

unread,
Feb 12, 2009, 3:39:50 PM2/12/09
to clo...@googlegroups.com
Stuart Sierra <the.stua...@gmail.com> writes:

> I could change "duck-streams/reader" to return a PushbackReader
> instead of a BufferedReader. Unfortunately, "readLine" is defined in
> BufferedReader but NOT in PushbackReader. We need "readLine" for
> "clojure.core/line-seq". So we can't have it both ways. Blame Java's
> over-engineered I/O.

I'm no Java IO expert, but it sounds like a BufferedReader is the right
thing in the majority of cases. I think having "read" wrap its stream in
a PushbackReader where necessary is a much better, non-intrusive
solution, and my implementation seems to work fine.

-Phil

Phil Hagelberg

unread,
Feb 12, 2009, 3:44:43 PM2/12/09
to clo...@googlegroups.com
Mark Fredrickson <mark.m.fr...@gmail.com> writes:

> While (read) taking an argument seems valid, I think you can do what
> you want in the short term using binding:

read actually already takes an argument, it just (without my patch)
requires it to be a PushbackReader.

> IIRC, duck-streams has a (with-reader ...) form that will handle the
> binding and close the reader when it exists (which may or may not be
> what you want).

I don't see with-reader in contrib anywhere. Maybe this comes from
somewhere else?

-Phil

Stephen C. Gilardi

unread,
Feb 12, 2009, 5:15:07 PM2/12/09
to clo...@googlegroups.com

On Feb 12, 2009, at 3:39 PM, Phil Hagelberg wrote:

> I'm no Java IO expert, but it sounds like a BufferedReader is the
> right
> thing in the majority of cases. I think having "read" wrap its
> stream in
> a PushbackReader where necessary is a much better, non-intrusive
> solution, and my implementation seems to work fine.

I don't think wrapping readers within read can work properly in all
cases. The problem is that the PushbackReader created within read may
end its life with a character still in its pushback buffer. If that
happens, subsequent clients of the wrapped reader should logically
receive that lost character first, but will instead receive the one
after it.

This problem wouldn't be immediately apparent in testing because
frequently that lost character would be whitespace. Losing it wouldn't
have a high-level effect.

Here's an example of where wrapping within read fails:

Before the patch is applied. read requires a PushbackReader, so I wrap
my reader in one and then read the two objects from the string.

user=> (def a (java.io.PushbackReader. (java.io.StringReader.
"123[145]")))
#'user/a
user=> (read a)
123
user=> (read a)
[145]

The two objects are read correctly.

Now I apply the patch and use the convenience it offers:

user=> (def a (java.io.StringReader. "123[145]"))
#'user/a
user=> (read a)
123
user=> (read a)
145

The "[" on the input stream was lost so the second object is read as a
number rather than a vector.

--Steve

Phil Hagelberg

unread,
Feb 12, 2009, 11:00:49 PM2/12/09
to clo...@googlegroups.com
"Stephen C. Gilardi" <sque...@mac.com> writes:

> Now I apply the patch and use the convenience it offers:
>
> user=> (def a (java.io.StringReader. "123[145]"))
> #'user/a
> user=> (read a)
> 123
> user=> (read a)
> 145
>
> The "[" on the input stream was lost so the second object is read as a
> number rather than a vector.

Yikes. I hadn't thought of that--I was only using read in cases where I
wanted to get a single object from the reader.

Taking things off the reader and then pushing them back on seems a
little odd to me--isn't there a way to just "peek" at the next character
without consuming from it? That's how I expected this to work.

Otherwise it seems to be a bit of a Gordian Knot.

-Phil

Stephen C. Gilardi

unread,
Feb 20, 2009, 6:07:39 PM2/20/09
to clo...@googlegroups.com

On Feb 12, 2009, at 11:00 PM, Phil Hagelberg wrote:

> Yikes. I hadn't thought of that--I was only using read in cases
> where I
> wanted to get a single object from the reader.
>
> Taking things off the reader and then pushing them back on seems a
> little odd to me--isn't there a way to just "peek" at the next
> character
> without consuming from it? That's how I expected this to work.

PushbackReader exists because pushing back one or a small number of
characters can make parsing text significantly easier. Rather than
forcing every input stream to allow some pushing back (or peeking),
PushbackReader encapsulates that capability and works with other
streams to provide it.

I see that issue 79 against Clojure requesting this change is still
open. If auto-wrapping of readers is no longer a change you'd like to
see, would you please either withdraw it (if you as the originator
have that capability) or add a comment to it requesting that it be
withdrawn?

Thanks,

--Steve

Phil Hagelberg

unread,
Feb 20, 2009, 6:35:47 PM2/20/09
to clo...@googlegroups.com
"Stephen C. Gilardi" <sque...@mac.com> writes:

> I see that issue 79 against Clojure requesting this change is still
> open. If auto-wrapping of readers is no longer a change you'd like to
> see, would you please either withdraw it (if you as the originator
> have that capability) or add a comment to it requesting that it be
> withdrawn?

Well I definitely don't think my patch will cut it, and I can't think of
another way around the problem.

Looks like I can't close the issue myself though, but I'm fine with
having it closed out.

-Phil

Reply all
Reply to author
Forward
0 new messages