patternrustModerate
Simple cat in Rust
Viewed 0 times
rustsimplecat
Problem
I'm learning Rust, from a Python background, and while I've used languages like C and C++ in the (distant) past, system languages aren't really my specialty.
I would just like to know if my code is sane, if I seem to have the principles roughly right, but I'd be happy to hear any improvements at all, particularly with performance. I'm also a bit unsure about error handling.
Anyway, for my first program I've made a simple implementation of the UNIX cat command, without support for any arguments:
I would just like to know if my code is sane, if I seem to have the principles roughly right, but I'd be happy to hear any improvements at all, particularly with performance. I'm also a bit unsure about error handling.
Anyway, for my first program I've made a simple implementation of the UNIX cat command, without support for any arguments:
use std::env;
use std::io;
use std::io::Read;
use std::io::Write;
use std::fs::File;
macro_rules! println_stderr(
($($arg:tt)*) => (
match writeln!(&mut io::stderr(), $($arg)* ) {
Ok(_) => {},
Err(err) => panic!("Unable to write to stderr: {}", err),
}
)
);
fn main() {
let args: Vec = env::args().collect();
if args.len() {},
Err(err) => {
println_stderr!("{}", err.to_string());
continue;
}
};
print!("{}", s);
}
} else {
for arg in &args[1..] {
let mut s = String::new();
let mut file = match File::open(arg) {
Ok(file) => file,
Err(err) => {
println_stderr!("{}", err.to_string());
continue;
}
};
match file.read_to_string(&mut s) {
Ok(_) => {},
Err(err) => {
println_stderr!("{}", err.to_string());
continue;
}
};
print!("{}", s);
}
}
}Solution
imports
Your imports should be compressed:
Although it might be better to use the
I prefer the former for its explicitness.
is better as just
The same occurs later, and in the macro.
prefer late initialization
You should only initialize
copy-paste error?
Your
plz no Unicode
You read and write strings holding the whole file (or line) -
plz no allocate
Using the above means we're stuck allocating 64k for each stream, even if there are many streams or the stream is line-buffered.
The first can be solved by passing the buffer into the function. The later can be solved by resizing the buffer up to some hard limit from a small size.
DRY
Note that there's still duplication between the
Making
This is more reasonable if we want to allow the code to accept
Result
Your imports should be compressed:
use std::io::{self, Read, Write};Although it might be better to use the
io prelude:use std::io;
use std::io::prelude::*;I prefer the former for its explicitness.
match vs if letmatch io::stdin().read_line(&mut s) {
Ok(_) => {},
Err(err) => {
println_stderr!("{}", err.to_string());
continue;
}
};is better as just
if let Err(err) = io::stdin().read_line(&mut s) {
println_stderr!("{}", err);
continue;
};The same occurs later, and in the macro.
prefer late initialization
You should only initialize
s just before use in the second branch.copy-paste error?
Your
continue when reading from stdin is a bit optimistic. I'd suggest just exiting at that point - if reading one line fails then reading the rest probably will too.plz no Unicode
You read and write strings holding the whole file (or line) -
cat should work instead on raw bytes, and preferrably in large chunks too. The most obvious way is something likefn redirect_stream(reader: &mut R, writer: &mut W, buffer: &mut Vec) -> io::Result
where R: Read, W: Write
{
let mut buffer = vec![0; 64 * 1024];
loop {
let len_read = try!(reader.read(&mut buffer));
if len_read == 0 {
return Ok(())
}
try!(writer.write_all(&buffer[..len_read]));
}
}plz no allocate
Using the above means we're stuck allocating 64k for each stream, even if there are many streams or the stream is line-buffered.
The first can be solved by passing the buffer into the function. The later can be solved by resizing the buffer up to some hard limit from a small size.
DRY
Note that there's still duplication between the
args.len() < 2 and else branches. One could solve this by writing a wrapping function to extract this functionality.fn handle_arg(reader: &mut R, writer: &mut W, buffer: &mut Vec)
where R: Read, W: Write
{
if let Err(err) = redirect_stream(reader, writer, buffer) {
println_stderr!("{}", err.to_string());
}
}Making
handle_arg a closure would be prettier but require dynamic dispatch. That's probably fine, but maybe less idiomatic:let stdout = &mut io::stdout();
let buffer = &mut vec![0; SMALL_BUFFER_SIZE];
let mut handle_arg = move |mut reader: &mut Read| {
if let Err(err) = redirect_stream(&mut reader, stdout, buffer) {
println_stderr!("{}", err.to_string());
}
};- means stdin?This is more reasonable if we want to allow the code to accept
- to mean standard input, as we can dolet mut args: Vec = env::args().skip(1).collect();
if args.is_empty() {
args.push("-".into());
}
let stdout = &mut io::stdout();
let buffer = &mut vec![0; SMALL_BUFFER_SIZE];
for arg in args {
if arg == "-" {
handle_arg(&mut io::stdin(), stdout, buffer);
continue;
}
match File::open(arg) {
Ok(ref mut file) => {
handle_arg(file, stdout, buffer)
},
Err(err) => {
println_stderr!("{}", err);
continue;
}
}
}Result
use std::env;
use std::io::{self, Read, Write};
use std::iter;
use std::fs::File;
const SMALL_BUFFER_SIZE: usize = 256;
const LARGE_BUFFER_SIZE: usize = 64 * 1024;
macro_rules! println_stderr(
($($arg:tt)*) => (
if let Err(err) = writeln!(&mut io::stderr(), $($arg)* ) {
panic!("Unable to write to stderr: {}", err);
}
)
);
fn redirect_stream(reader: &mut R, writer: &mut W, buffer: &mut Vec) -> io::Result
where R: Read, W: Write
{
loop {
let len_read = try!(reader.read(buffer));
if len_read == 0 {
return Ok(())
}
try!(writer.write_all(&buffer[..len_read]));
if len_read == buffer.len() && len_read = env::args().skip(1).collect();
if args.is_empty() {
args.push("-".into());
}
fn handle_arg(reader: &mut R, writer: &mut W, buffer: &mut Vec)
where R: Read, W: Write
{
if let Err(err) = redirect_stream(reader, writer, buffer) {
println_stderr!("{}", err.to_string());
}
}
let stdout = &mut io::stdout();
let buffer = &mut vec![0; SMALL_BUFFER_SIZE];
for arg in args {
if arg == "-" {
handle_arg(&mut io::stdin(), stdout, buffer);
continue;
}
match File::open(arg) {
Ok(ref mut file) => {
handle_arg(file, stdout, buffer)
},
Err(err) => {
println_stderr!("{}", err);
continue;
}
}
}
}Code Snippets
use std::io::{self, Read, Write};use std::io;
use std::io::prelude::*;match io::stdin().read_line(&mut s) {
Ok(_) => {},
Err(err) => {
println_stderr!("{}", err.to_string());
continue;
}
};if let Err(err) = io::stdin().read_line(&mut s) {
println_stderr!("{}", err);
continue;
};fn redirect_stream<R, W>(reader: &mut R, writer: &mut W, buffer: &mut Vec<u8>) -> io::Result<()>
where R: Read, W: Write
{
let mut buffer = vec![0; 64 * 1024];
loop {
let len_read = try!(reader.read(&mut buffer));
if len_read == 0 {
return Ok(())
}
try!(writer.write_all(&buffer[..len_read]));
}
}Context
StackExchange Code Review Q#94941, answer score: 16
Revisions (0)
No revisions yet.