Function design opinions

code review
1

Say you were to design a function like (examples from std and serde) :

fn copy<R, W>(reader: R, writer: W) -> ResultOfSomeKind
where
    R: std::io::Read,
    W: std::io::Write;

would you prefer the signature above or :

fn copy<R, W>(reader: &mut R, writer: &mut W) -> ResultOfSomeKind
where
    R: std::io::Read,
    W: std::io::Write;

which, as a user, would you prefer ?

2 Likes
2

The first signature is strictly more general than the second, so I'd prefer the first one.

3 Likes
3

In particular, it is a real pain having to pass mutable references when a non-reference value would do, and Read and Write are blanket-impl'd for reference types anyway. So yeah, just use the first one.

1 Like
4

Note that functions with the first signature cannot be called recursively and repeatedly.

  • convert_stream_as::<R, W, T>(reader, writer) -> io::Result<()> cannot call convert_stream_as(reader, writer) itself more than once inside it, since the first recursive call moves ownership of the reader and writer to the callee.
  • Calling convert_stream_as<_, _, T>(&mut reader, &mut writer) also does not compile because it depends on more deeply nested convert_stream_as() infinitely.
    • convert_stream_as::<R, W, T> depends on convert_stream_as::<&mut R, &mut W, T>(), and it depends on convert_stream_as::<&mut &mut R, &mut &mut W, T>(), and so on.

On the other hand, reader: &mut R, writer: &mut W version allows recursive call (example on playground).

This might be problem if you want to define, for example, serializer and deserializer for complex nested types such as tree structure.
If this restriction won't affect your use case (for now and in future), prefer reader: R, writer: W version.

3 Likes
5

Okay, I noticed that you can make recursively called functions receive reader: R, writer: W, by making it proxy to the reader: &mut R, writer: &mut W version.
You can always use reader: R, writer: W. Prefer it for public API.

use std::io::{self, Read, Write};

// Public API.
#[inline]
pub fn foo<R: Read, W: Write>(mut reader: R, mut writer: W, depth: usize) -> io::Result<()> {
    foo_impl(&mut reader, &mut writer, depth)
}

// Not public.
fn foo_impl<R: Read, W: Write>(reader: &mut R, writer: &mut W, depth: usize) -> io::Result<()> {
    if depth != 0 {
        // Recursive call is possible.
        foo_impl(reader, writer, depth - 1)?;
        foo_impl(reader, writer, depth - 1)?;
    }
    Ok(())
}

(playground)

This would cause extra dereference when R and W is a reference to some other reader / writer types, but I don't know how much it costs and how well the compiler optimizes them.

3 Likes
6

I think there's a consensus¹ towards the first signature, which was also my instinct. Thank you.

¹: of 3 expressed opinions, but I would think many people abstained to see what was said, that's what I did

7

In Tokio we decided to go for the second version since this mirrors what std does.

1 Like
8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.