writer: Borrow values instead of requiring ownership

Author: Turbo87

README.md

@@ -48,7 +48,9 @@ use seeyou_cupx::cup::CupFile;
 use seeyou_cupx::CupxWriter;
 use std::path::Path;
 
-CupxWriter::new(CupFile::default())
+let cup_file = CupFile::default();
+
+CupxWriter::new(&cup_file)
     .add_picture("airport.jpg", Path::new("images/airport.jpg"))
     .add_picture("runway.jpg", Path::new("images/runway.jpg"))
     .write_to_path("output.cupx")?;

benches/cupx_writing.rs

@@ -1,18 +1,17 @@
-use criterion::{criterion_group, criterion_main, Criterion};
+use criterion::{Criterion, criterion_group, criterion_main};
 use seeyou_cup::CupFile;
 use seeyou_cupx::CupxWriter;
-use std::hint::black_box;
 use std::io::Cursor;
-use std::path::PathBuf;
+use std::path::Path;
 
 fn bench_write_empty(c: &mut Criterion) {
     c.bench_function("CupxWriter::write (empty)", |b| {
         let mut buffer = Vec::with_capacity(10_000);
         b.iter(|| {
             buffer.clear();
-            let writer = CupxWriter::new(CupFile::default());
-            black_box(writer.write(Cursor::new(&mut buffer)).unwrap());
-            black_box(buffer.len())
+            let cup_file = CupFile::default();
+            let writer = CupxWriter::new(&cup_file);
+            writer.write(Cursor::new(&mut buffer)).unwrap();
         })
     });
 }
@@ -24,10 +23,10 @@ fn bench_write_with_single_picture(c: &mut Criterion) {
         let mut buffer = Vec::with_capacity(50_000);
         b.iter(|| {
             buffer.clear();
-            let mut writer = CupxWriter::new(CupFile::default());
-            writer.add_picture("test.jpg", picture_data.clone());
-            black_box(writer.write(Cursor::new(&mut buffer)).unwrap());
-            black_box(buffer.len())
+            let cup_file = CupFile::default();
+            let mut writer = CupxWriter::new(&cup_file);
+            writer.add_picture("test.jpg", picture_data.as_slice());
+            writer.write(Cursor::new(&mut buffer)).unwrap();
         })
     });
 }
@@ -37,10 +36,10 @@ fn bench_write_with_picture_from_path(c: &mut Criterion) {
         let mut buffer = Vec::with_capacity(50_000);
         b.iter(|| {
             buffer.clear();
-            let mut writer = CupxWriter::new(CupFile::default());
-            writer.add_picture("2_1034.jpg", PathBuf::from("tests/fixtures/2_1034.jpg"));
-            black_box(writer.write(Cursor::new(&mut buffer)).unwrap());
-            black_box(buffer.len())
+            let cup_file = CupFile::default();
+            let mut writer = CupxWriter::new(&cup_file);
+            writer.add_picture("2_1034.jpg", Path::new("tests/fixtures/2_1034.jpg"));
+            writer.write(Cursor::new(&mut buffer)).unwrap();
         })
     });
 }
@@ -54,12 +53,12 @@ fn bench_write_with_multiple_pictures(c: &mut Criterion) {
         let mut buffer = Vec::with_capacity(200_000);
         b.iter(|| {
             buffer.clear();
-            let mut writer = CupxWriter::new(CupFile::default());
-            writer.add_picture("small.jpg", picture_data_small.clone());
-            writer.add_picture("medium.jpg", picture_data_medium.clone());
-            writer.add_picture("large.jpg", picture_data_large.clone());
-            black_box(writer.write(Cursor::new(&mut buffer)).unwrap());
-            black_box(buffer.len())
+            let cup_file = CupFile::default();
+            let mut writer = CupxWriter::new(&cup_file);
+            writer.add_picture("small.jpg", picture_data_small.as_slice());
+            writer.add_picture("medium.jpg", picture_data_medium.as_slice());
+            writer.add_picture("large.jpg", picture_data_large.as_slice());
+            writer.write(Cursor::new(&mut buffer)).unwrap();
         })
     });
 }

src/writer.rs

@@ -3,7 +3,7 @@ use seeyou_cup::CupFile;
 use std::collections::HashMap;
 use std::fs::File;
 use std::io::{Cursor, Seek, Write};
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 /// A builder for creating CUPX files with waypoint data and pictures.
 ///
@@ -18,46 +18,41 @@ use std::path::{Path, PathBuf};
 /// use seeyou_cup::CupFile;
 /// # use std::path::Path;
 ///
-/// CupxWriter::new(CupFile::default())
+/// # let cup_file = CupFile::default();
+/// CupxWriter::new(&cup_file)
 ///     .add_picture("photo.jpg", Path::new("images/photo.jpg"))
 ///     .write_to_path("output.cupx")?;
 /// # Ok::<(), seeyou_cupx::Error>(())
 /// ```
-pub struct CupxWriter {
-    cup_file: CupFile,
-    pictures: HashMap<String, PictureSource>,
+pub struct CupxWriter<'a> {
+    cup_file: &'a CupFile,
+    pictures: HashMap<&'a str, PictureSource<'a>>,
 }
 
 /// Source of picture data for inclusion in a CUPX file.
 ///
-/// Pictures can be provided either as in-memory byte vectors or as file paths
+/// Pictures can be provided either as in-memory byte slices or as file paths
 /// that will be read when the CUPX file is written.
-pub enum PictureSource {
-    /// Picture data provided as a byte vector in memory.
-    Bytes(Vec<u8>),
+pub enum PictureSource<'a> {
+    /// Picture data provided as a borrowed byte slice.
+    Bytes(&'a [u8]),
     /// Picture data will be read from a file at the given path.
-    Path(PathBuf),
+    Path(&'a Path),
 }
 
-impl From<Vec<u8>> for PictureSource {
-    fn from(bytes: Vec<u8>) -> Self {
+impl<'a> From<&'a [u8]> for PictureSource<'a> {
+    fn from(bytes: &'a [u8]) -> Self {
         PictureSource::Bytes(bytes)
     }
 }
 
-impl From<PathBuf> for PictureSource {
-    fn from(path: PathBuf) -> Self {
+impl<'a> From<&'a Path> for PictureSource<'a> {
+    fn from(path: &'a Path) -> Self {
         PictureSource::Path(path)
     }
 }
 
-impl From<&Path> for PictureSource {
-    fn from(path: &Path) -> Self {
-        PictureSource::Path(path.to_path_buf())
-    }
-}
-
-impl CupxWriter {
+impl<'a> CupxWriter<'a> {
     /// Creates a new CUPX writer with the given waypoint/task data.
     ///
     /// Pictures can be added using [`add_picture`](Self::add_picture).
@@ -69,10 +64,10 @@ impl CupxWriter {
     /// use seeyou_cup::CupFile;
     ///
     /// let cup_file = CupFile::default();
-    /// let writer = CupxWriter::new(cup_file);
+    /// let writer = CupxWriter::new(&cup_file);
     /// # Ok::<(), seeyou_cupx::Error>(())
     /// ```
-    pub fn new(cup_file: CupFile) -> Self {
+    pub fn new(cup_file: &'a CupFile) -> Self {
         Self {
             cup_file,
             pictures: HashMap::new(),
@@ -93,18 +88,20 @@ impl CupxWriter {
     /// use seeyou_cup::CupFile;
     /// # use std::path::Path;
     ///
-    /// CupxWriter::new(CupFile::default())
+    /// # let cup_file = CupFile::default();
+    /// # let image_data = vec![0u8; 100];
+    /// CupxWriter::new(&cup_file)
     ///     .add_picture("photo1.jpg", Path::new("images/photo1.jpg"))
-    ///     .add_picture("photo2.jpg", vec![0u8; 100])
+    ///     .add_picture("photo2.jpg", &image_data[..])
     ///     .write_to_path("output.cupx")?;
     /// # Ok::<(), seeyou_cupx::Error>(())
     /// ```
     pub fn add_picture(
         &mut self,
-        filename: impl Into<String>,
-        source: impl Into<PictureSource>,
+        filename: &'a str,
+        source: impl Into<PictureSource<'a>>,
     ) -> &mut Self {
-        self.pictures.insert(filename.into(), source.into());
+        self.pictures.insert(filename, source.into());
         self
     }
 
@@ -121,7 +118,7 @@ impl CupxWriter {
     pub fn write<W: Write + Seek>(&self, writer: W) -> Result<(), Error> {
         for filename in self.pictures.keys() {
             if filename.is_empty() || filename.contains('/') || filename.contains('\\') {
-                return Err(Error::InvalidFilename(filename.clone()));
+                return Err(Error::InvalidFilename(filename.to_string()));
             }
         }
 
@@ -168,7 +165,8 @@ impl CupxWriter {
     /// use seeyou_cupx::CupxWriter;
     /// use seeyou_cup::CupFile;
     ///
-    /// let bytes = CupxWriter::new(CupFile::default()).write_to_vec()?;
+    /// let cup_file = CupFile::default();
+    /// let bytes = CupxWriter::new(&cup_file).write_to_vec()?;
     /// # Ok::<(), seeyou_cupx::Error>(())
     /// ```
     ///
@@ -190,7 +188,8 @@ impl CupxWriter {
     /// use seeyou_cupx::CupxWriter;
     /// use seeyou_cup::CupFile;
     ///
-    /// CupxWriter::new(CupFile::default()).write_to_path("output.cupx")?;
+    /// let cup_file = CupFile::default();
+    /// CupxWriter::new(&cup_file).write_to_path("output.cupx")?;
     /// # Ok::<(), seeyou_cupx::Error>(())
     /// ```
     ///

tests/large_cup_file_test.rs

@@ -18,8 +18,8 @@ fn test_large_cup_file() {
     }
 
     // Write the CUPX file with a small picture to ensure we have two ZIP archives
-    let cupx_buffer = CupxWriter::new(cup_file)
-        .add_picture("test.jpg", b"small test image data".to_vec())
+    let cupx_buffer = CupxWriter::new(&cup_file)
+        .add_picture("test.jpg", &b"small test image data"[..])
         .write_to_vec()
         .unwrap();
 

tests/writer_test.rs

@@ -2,11 +2,12 @@ use insta::{assert_binary_snapshot, assert_compact_debug_snapshot};
 use seeyou_cupx::cup::CupFile;
 use seeyou_cupx::{CupxFile, CupxWriter};
 use std::io::{Cursor, Read};
-use std::path::PathBuf;
+use std::path::Path;
 
 #[test]
 fn test_write_empty() {
-    let buffer = CupxWriter::new(CupFile::default()).write_to_vec().unwrap();
+    let cup_file = CupFile::default();
+    let buffer = CupxWriter::new(&cup_file).write_to_vec().unwrap();
 
     let (result, _) = CupxFile::from_reader(Cursor::new(&buffer)).unwrap();
     assert_eq!(result.waypoints().len(), 0);
@@ -16,10 +17,11 @@ fn test_write_empty() {
 
 #[test]
 fn test_write_with_bytes_picture() {
+    let cup_file = CupFile::default();
     let picture_data = b"fake image data".to_vec();
 
-    let buffer = CupxWriter::new(CupFile::default())
-        .add_picture("test.jpg", picture_data.clone())
+    let buffer = CupxWriter::new(&cup_file)
+        .add_picture("test.jpg", &picture_data[..])
         .write_to_vec()
         .unwrap();
 
@@ -38,8 +40,9 @@ fn test_write_with_bytes_picture() {
 
 #[test]
 fn test_write_with_path_picture() {
-    let buffer = CupxWriter::new(CupFile::default())
-        .add_picture("2_1034.jpg", PathBuf::from("tests/fixtures/2_1034.jpg"))
+    let cup_file = CupFile::default();
+    let buffer = CupxWriter::new(&cup_file)
+        .add_picture("2_1034.jpg", Path::new("tests/fixtures/2_1034.jpg"))
         .write_to_vec()
         .unwrap();
 
@@ -58,12 +61,13 @@ fn test_write_with_path_picture() {
 
 #[test]
 fn test_write_duplicate_filename_replaces() {
+    let cup_file = CupFile::default();
     let first_data = b"first".to_vec();
     let second_data = b"second".to_vec();
 
-    let buffer = CupxWriter::new(CupFile::default())
-        .add_picture("test.jpg", first_data)
-        .add_picture("test.jpg", second_data.clone())
+    let buffer = CupxWriter::new(&cup_file)
+        .add_picture("test.jpg", &first_data[..])
+        .add_picture("test.jpg", &second_data[..])
         .write_to_vec()
         .unwrap();
 
@@ -82,10 +86,11 @@ fn test_write_duplicate_filename_replaces() {
 
 #[test]
 fn test_write_multiple_pictures() {
-    let buffer = CupxWriter::new(CupFile::default())
-        .add_picture("a.jpg", b"data a".to_vec())
-        .add_picture("b.jpg", b"data b".to_vec())
-        .add_picture("c.jpg", b"data c".to_vec())
+    let cup_file = CupFile::default();
+    let buffer = CupxWriter::new(&cup_file)
+        .add_picture("a.jpg", &b"data a"[..])
+        .add_picture("b.jpg", &b"data b"[..])
+        .add_picture("c.jpg", &b"data c"[..])
         .write_to_vec()
         .unwrap();
 
@@ -97,46 +102,51 @@ fn test_write_multiple_pictures() {
 
 #[test]
 fn test_write_invalid_filename_empty() {
-    let result = CupxWriter::new(CupFile::default())
-        .add_picture("", b"data".to_vec())
+    let cup_file = CupFile::default();
+    let result = CupxWriter::new(&cup_file)
+        .add_picture("", &b"data"[..])
         .write_to_vec();
 
     assert_compact_debug_snapshot!(result, @r#"Err(InvalidFilename(""))"#);
 }
 
 #[test]
 fn test_write_invalid_filename_with_slash() {
-    let result = CupxWriter::new(CupFile::default())
-        .add_picture("path/to/file.jpg", b"data".to_vec())
+    let cup_file = CupFile::default();
+    let result = CupxWriter::new(&cup_file)
+        .add_picture("path/to/file.jpg", &b"data"[..])
         .write_to_vec();
 
     assert_compact_debug_snapshot!(result, @r#"Err(InvalidFilename("path/to/file.jpg"))"#);
 }
 
 #[test]
 fn test_write_invalid_filename_with_backslash() {
-    let result = CupxWriter::new(CupFile::default())
-        .add_picture("path\\to\\file.jpg", b"data".to_vec())
+    let cup_file = CupFile::default();
+    let result = CupxWriter::new(&cup_file)
+        .add_picture("path\\to\\file.jpg", &b"data"[..])
         .write_to_vec();
 
     assert_compact_debug_snapshot!(result, @r#"Err(InvalidFilename("path\\to\\file.jpg"))"#);
 }
 
 #[test]
 fn test_write_nonexistent_path() {
-    let result = CupxWriter::new(CupFile::default())
-        .add_picture("test.jpg", PathBuf::from("nonexistent/file.jpg"))
+    let cup_file = CupFile::default();
+    let result = CupxWriter::new(&cup_file)
+        .add_picture("test.jpg", Path::new("nonexistent/file.jpg"))
         .write_to_vec();
 
     assert_compact_debug_snapshot!(result, @r#"Err(Io(Os { code: 2, kind: NotFound, message: "No such file or directory" }))"#);
 }
 
 #[test]
 fn test_write_to_path() {
+    let cup_file = CupFile::default();
     let temp_path = std::env::temp_dir().join("test_cupx_writer.cupx");
 
-    CupxWriter::new(CupFile::default())
-        .add_picture("test.jpg", b"test data".to_vec())
+    CupxWriter::new(&cup_file)
+        .add_picture("test.jpg", &b"test data"[..])
         .write_to_path(&temp_path)
         .unwrap();