[m-rev.] For review: Java implementation of IO library

[Fergus Henderson]

On 14-Jan-2004, James Goddard <goddardjames at yahoo.com> wrote:
> Index: io.m
> @@ -4615,6 +4818,332 @@
>  }
>  ").
>  
> +:- pragma foreign_code("Java",
> +"
> +	// stdin, stdout, stderr and all text files are run through stream
> +	// readers/writers, which use the default charset encoding.  In other
> +	// words, while strings and characters are stored internally using
> +	// unicode, all files are read and written using the default system
> +	// charset.
> +	// Binary files are opened via RandomAccessFile, which supports seek,
> +	// but not character encoding/decoding or buffering.
> +	
> +	public static class MR_MercuryFileStruct {
> +		public	static int			ML_next_stream_id = 0;
> +		public	int				id;
> +
> +		public	int				line_number;
> +
> +		public	static final int		INPUT		= 0;
> +		public	static final int		OUTPUT		= 1;
> +		private int				mode;
> +
> +		private static final int		SEEK_SET	= 0;
> +		private static final int		SEEK_CUR	= 1;
> +		private static final int		SEEK_END	= 2;
> +
> +		private	java.util.Stack			pushback	= null;
> +		private java.io.InputStreamReader	input		= null;
> +		private java.io.OutputStreamWriter	output		= null;
> +		private java.io.RandomAccessFile	randomaccess	= null;
> +
> +		private java.lang.String		filename	= null;
> +
> +		public MR_MercuryFileStruct(java.io.InputStream stream) {
> +			line_number	= 1;
> +			id		= ML_next_stream_id++;
> +			mode		= INPUT;
> +			pushback	= new java.util.Stack();
> +			input		= new java.io.InputStreamReader(
> +							stream);
> +		}
> +
> +		public MR_MercuryFileStruct(java.io.OutputStream stream) {
> +			line_number	= 1;
> +			id		= ML_next_stream_id++;
> +			mode		= OUTPUT;
> +			output		= new java.io.OutputStreamWriter(
> +							stream);
> +		}
> +
> +		public MR_MercuryFileStruct(java.lang.String file, char mode) {
> +			line_number	= 1;
> +			id		= ML_next_stream_id++;
> +			filename	= file;
> +			String openstring;
> +			if (mode == 'r') {
> +				openstring = ""r"";
> +				this.mode = INPUT;
> +				pushback = new java.util.Stack();
> +			} else if (mode == 'r' || mode == 'a') {
> +				openstring = ""rw"";

s/'r'/'w'/

This change obviously needs more testing ;-)

> +		public int size() {
> +			try {
> +				return (int) randomaccess.length();
> +			} catch (java.lang.Exception e) {
> +				return -1;
> +			}
> +		}

Please document that this function only works for binary files.
(For text files, randomaccess will be null.)

> +		public void seek(int flag, int offset) {
> +			if (randomaccess == null) {
> +				throw new java.lang.RuntimeException(
> +						""Stream not seekable"");
> +				// this applies to stdin, stdout and stderr
> +			}

The comment here should be updated -- it applies to any text files,
doesn't it?

> +		public int getOffset() {

Please document that this function only works for binary files.

> +		/* read_char():
> +		**	Reads one character in using the default charset
> +		**	decoding.  Returns -1 at end of file.
> +		*/

The "/*" should be on a line of its own.

		/*
		** read_char():
		**      ...
		*/

Also, s/Reads one character/Reads one character from a text input file/

> +		/* read_char():
> +		**	Reads one byte in from a binary file.
> +		**	Returns -1 at end of file.
> +		*/
> +		public int read_byte() {

Likewise.  Also s/read_char/read_byte/

> +		public void ungetc(int c) {
> +			if (mode == OUTPUT) {
> +				throw new java.lang.RuntimeException(
> +				""Attempted to unget char to output stream"");
> +			}
> +			if (c == '\\n') {
> +				line_number--;
> +			}
> +
> +			pushback.push(new Integer(c));
> +		}

It's still not clear whether the parameter `c' here is supposed to represent
a character or a byte.  Please document this.

If the idea is that it can be used for pushing back either bytes
or characters, depending on whether the stream is a text stream or
a binary stream, then please document that.  But I don't know how
that would work with io__write_binary and io__read_binary.

> +		public void putc(int c) {

Likewise here.

> +		public void close() {
> +			try {
> +				if (input != null) {
> +					input.close();
> +				} else if (output != null) {
> +					output.close();
> +				} else {
> +					randomaccess.close();
> +				}

I suggest deleting the elses here, so that this code will still work if
more than one of input/output/randomaccess is non-null.

> +	// StreamPipe is a mechanism for connecting streams to those of a
> +	// Runtime.exec() Process.
> +
> +	private static class StreamPipe extends java.lang.Thread {
> +		MR_MercuryFileStruct in;
> +		MR_MercuryFileStruct out;
> +		boolean live			= true;
> +		boolean closeOutput		= false;
> +		java.lang.Exception exception	= null;
> +
> +		StreamPipe(java.io.InputStream in, MR_MercuryFileStruct out) {
> +			this.in  = new MR_MercuryFileStruct(in);
> +			this.out = out;
> +		}
> +
> +		StreamPipe(MR_MercuryFileStruct in, java.io.OutputStream out) {
> +			this.in  = in;
> +			this.out = new MR_MercuryFileStruct(out);
> +			closeOutput = true;
> +		}
> +
> +		public void run() {
> +			try {
> +				while (live) {
> +					int b = in.read_char();
> +					if (b == -1) {
> +						break;
> +					}
> +					out.putc(b);
> +				}
> +				out.flush();
> +				if (closeOutput) {
> +					out.close();
> +				}
> +			}
> +			catch (java.lang.Exception e) {
> +				exception = e;
> +			}
> +		}
> +	} // class StreamPipe
> +").

It looks like "live" is never set to anything but "true".

> +:- pragma foreign_code("Java",
> +"
> +static MR_MercuryFileStruct mercury_stdin =
> +		new MR_MercuryFileStruct(java.lang.System.in);
> +static MR_MercuryFileStruct mercury_stdout =
> +		new MR_MercuryFileStruct(java.lang.System.out);
> +static MR_MercuryFileStruct mercury_stderr =
> +		new MR_MercuryFileStruct(java.lang.System.err);
> +static MR_MercuryFileStruct mercury_stdin_binary =
> +		new MR_MercuryFileStruct(java.lang.System.in);
> +static MR_MercuryFileStruct mercury_stdout_binary =
> +		new MR_MercuryFileStruct(java.lang.System.out);

Here there seems to be no distinction between mercury_stdin and
mercury_stdin_binary.

mercury_stdin_binary and mercury_stdout_binary should be opened in
binary mode; in particular, it should be possible to read/write
binary data (bytes), and also to seek on them.

> +static java.lang.Exception MR_io_exception;

Please add a comment "XXX not thread-safe!".

> +:- pragma foreign_proc("Java",
> +	io__write_bytes(Message::in, _IO0::di, _IO::uo),
> +	[may_call_mercury, promise_pure, thread_safe, tabled_for_io],
> +"{
> +	mercury_current_binary_output.write(Message);
> +}").

It would probably be better for write_bytes in Java to behave the
same way it does in .NET.  See the long comment in the C# version
of mercury_print_binary_string().

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------