Unify codes using std::fmt and std::io

Hello,

This gives me a "duplicated code" warning. write_iter_fmt is used inside Display implementations, write_iter_io everywhere else. Any way to avoid the duplication? Thanks!

fn write_iter_fmt<T: Display, W: std::fmt::Write>(w: &mut W, iter: &mut Iter<T>, sep: &str)
                                                  -> std::fmt::Result {
    if let Some(first) = iter.next() {
        write!(w, "{}", first)?;
        for item in iter {
            write!(w, "{}{}", sep, item)?;
        }
    }
    Ok(())
}
fn write_iter_io<T: Display, W: std::io::Write>(w: &mut W, iter: &mut Iter<T>, sep: &str)
                                                -> std::io::Result<()> {
    if let Some(first) = iter.next() {
        write!(w, "{}", first)?;
        for item in iter {
            write!(w, "{}{}", sep, item)?;
        }
    }
    Ok(())
}
1 Like

I'd probably just use a macro TBH.

My architecture / coding style for formatting routines like this is:

  • All formatting is expressed by implementing a formatting trait (or returning an existing implementation) — so in this case, you would define

    struct DisplayIter(pub I); // simple wrapper type
    impl<I: Iterator> fmt::Display for DisplayIter<I> { ...
    

    The only free functions that accept fmt::Write should be private helper functions for these trait implementations.

  • Now, you can adapt this to io::Write simply:

    write!(some_io_write, "{}", DisplayIter(&my_iterator))?;
    

    (Internally, the actual adaptation happens in std::io::Write::write_fmt(), the method that write!() calls.)

As long as you don't write a public function that demands fmt::Write, you won't encounter a need to have a separate one for io::Write. I feel that this approach is elegant and makes the best use of the flexibility already provided by the standard library.

3 Likes

Note though that with a &I where I: Iterator you can't really do much, since iterating it requires an &mut reference. Storing I or &mut I won't help either because Display::fmt takes a &self. You'll have to do something like storing a Cell<Option<I>> and .take() it.

I'm sorry, I made a think-o — that should have been ownership, and I briefly thought “well we're just formatting, so we don't need to take ownership”, but no. Fixed. [Edit again: or not fixed, due to the &self issue you point out.]

This does make things a little weird since the DisplayIter value can only be printed once.

A better idea might be to keep the original write_iter_fmt function, and say “This should only be used from a formatting trait implementation that creates the iterator to be formatted”.

Wow, thanks for al the feedback! This was very helpful.

I came up with this - at least, no more duplicated code warning!

fn join_iter<T, I, E, P>(sep: &str, mut iter: I, mut print_item: P) -> Result<(), E>
where I: Iterator<Item=T>, P: FnMut(&str, T) -> Result<(), E>
{
    if let Some(first) = iter.next() {
        print_item("", first)?;
        for item in iter {
            print_item(sep, item)?;
        }
    }
    Ok(())
}

pub(crate) fn write_iter_fmt<T, I, W>(w: &mut W, iter: I, sep: &str) -> std::fmt::Result
where T: Display, I: Iterator<Item=T>, W: std::fmt::Write
{
    join_iter(sep, iter, |sep, item| write!(w, "{}{}", sep, item))
}

pub(crate) fn write_iter_io<T, I, W>(w: &mut W, iter: I, sep: &str) -> std::io::Result<()>
where T: Display, I: Iterator<Item=T>, W: std::io::Write
{
    join_iter(sep, iter, |sep, item| write!(w, "{}{}", sep, item))
}

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.