From ba0ea3f221e1cc2ffd9c3f5d77f6c456336af0a1 Mon Sep 17 00:00:00 2001 From: glitchySid Date: Thu, 8 Jan 2026 23:42:10 +0530 Subject: [PATCH] perf: eliminate unnecessary clones and improve API ergonomics - PromptBuilder::new now takes &[String] instead of Vec - GeminiClient::new now takes &str, &[String] instead of owned values - FileBatch::from_path now takes &Path instead of PathBuf - categorize_files_offline now takes Vec (ownership) instead of &[String] - handle_offline_organization now takes FileBatch by value These changes eliminate ~5-50 KB of unnecessary allocations for typical file counts, reduce allocator pressure, and improve API clarity by properly expressing ownership semantics. No functional changes - all tests pass. --- src/cli/handlers/offline.rs | 4 ++-- src/cli/handlers/online.rs | 2 +- src/cli/orchestrator.rs | 6 +++--- src/files/batch.rs | 13 +++++++------ src/files/categorizer.rs | 10 +++++----- src/gemini/client.rs | 11 +++++++---- src/gemini/prompt.rs | 2 +- src/settings/prompt.rs | 9 ++++++--- 8 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/cli/handlers/offline.rs b/src/cli/handlers/offline.rs index 6b42bde..e121554 100644 --- a/src/cli/handlers/offline.rs +++ b/src/cli/handlers/offline.rs @@ -6,14 +6,14 @@ use std::collections::HashMap; use std::path::Path; pub fn handle_offline_organization( - batch: &FileBatch, + batch: FileBatch, target_path: &Path, dry_run: bool, undo_log: &mut UndoLog, ) -> Result, Box> { println!("{}", "Categorizing files by extension...".cyan()); - let result = categorize_files_offline(&batch.filenames); + let result = categorize_files_offline(batch.filenames); if result.plan.files.is_empty() { println!("{}", "No files could be categorized offline.".yellow()); diff --git a/src/cli/handlers/online.rs b/src/cli/handlers/online.rs index e5f0284..09dc838 100644 --- a/src/cli/handlers/online.rs +++ b/src/cli/handlers/online.rs @@ -18,7 +18,7 @@ pub async fn handle_online_organization( cache: &mut Cache, undo_log: &mut UndoLog, ) -> Result, Box> { - let client = GeminiClient::new(config.api_key.clone(), config.categories.clone()); + let client = GeminiClient::new(&config.api_key, &config.categories); println!("Asking Gemini to organize..."); diff --git a/src/cli/orchestrator.rs b/src/cli/orchestrator.rs index 17b3b9a..b3f80c2 100644 --- a/src/cli/orchestrator.rs +++ b/src/cli/orchestrator.rs @@ -42,7 +42,7 @@ pub async fn handle_organization( } }; - let batch = FileBatch::from_path(target_path.clone(), args.recursive); + let batch = FileBatch::from_path(&target_path, args.recursive); if batch.filenames.is_empty() { println!("{}", "No files found to organize!".yellow()); @@ -56,7 +56,7 @@ pub async fn handle_organization( println!("{}", "Using offline mode (--offline flag).".cyan()); true } else { - let client = GeminiClient::new(config.api_key.clone(), config.categories.clone()); + let client = GeminiClient::new(&config.api_key, &config.categories); match client.check_connectivity().await { Ok(()) => false, Err(e) => { @@ -71,7 +71,7 @@ pub async fn handle_organization( }; let plan = if use_offline { - handle_offline_organization(&batch, &target_path, args.dry_run, &mut undo_log)? + handle_offline_organization(batch, &target_path, args.dry_run, &mut undo_log)? } else { handle_online_organization( &args, diff --git a/src/files/batch.rs b/src/files/batch.rs index f111446..e0ad951 100644 --- a/src/files/batch.rs +++ b/src/files/batch.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use walkdir::WalkDir; #[derive(Debug)] @@ -8,7 +8,7 @@ pub struct FileBatch { } impl FileBatch { - pub fn from_path(root_path: PathBuf, recursive: bool) -> Self { + pub fn from_path(root_path: &Path, recursive: bool) -> Self { let mut filenames = Vec::new(); let mut paths = Vec::new(); let walker = if recursive { @@ -45,6 +45,7 @@ impl FileBatch { mod tests { use super::*; use std::fs::{self, File}; + use std::path::Path; #[test] fn test_file_batch_from_path() { @@ -55,7 +56,7 @@ mod tests { File::create(dir_path.join("file2.rs")).unwrap(); fs::create_dir(dir_path.join("subdir")).unwrap(); - let batch = FileBatch::from_path(dir_path.to_path_buf(), false); + let batch = FileBatch::from_path(dir_path, false); assert_eq!(batch.count(), 2); assert!(batch.filenames.contains(&"file1.txt".to_string())); assert!(batch.filenames.contains(&"file2.rs".to_string())); @@ -63,7 +64,7 @@ mod tests { #[test] fn test_file_batch_from_path_nonexistent() { - let batch = FileBatch::from_path(PathBuf::from("/nonexistent/path"), false); + let batch = FileBatch::from_path(Path::new("/nonexistent/path"), false); assert_eq!(batch.count(), 0); } @@ -75,7 +76,7 @@ mod tests { File::create(dir_path.join("file2.rs")).unwrap(); fs::create_dir(dir_path.join("subdir")).unwrap(); File::create(dir_path.join("subdir").join("file3.txt")).unwrap(); - let batch = FileBatch::from_path(dir_path.to_path_buf(), false); + let batch = FileBatch::from_path(dir_path, false); assert_eq!(batch.count(), 2); assert!(batch.filenames.contains(&"file1.txt".to_string())); assert!(batch.filenames.contains(&"file2.rs".to_string())); @@ -93,7 +94,7 @@ mod tests { File::create(dir_path.join("subdir1").join("nested").join("file3.md")).unwrap(); fs::create_dir(dir_path.join("subdir2")).unwrap(); File::create(dir_path.join("subdir2").join("file4.py")).unwrap(); - let batch = FileBatch::from_path(dir_path.to_path_buf(), true); + let batch = FileBatch::from_path(dir_path, true); assert_eq!(batch.count(), 4); assert!(batch.filenames.contains(&"file1.txt".to_string())); assert!(batch.filenames.contains(&"subdir1/file2.rs".to_string())); diff --git a/src/files/categorizer.rs b/src/files/categorizer.rs index 9e3017d..c04541b 100644 --- a/src/files/categorizer.rs +++ b/src/files/categorizer.rs @@ -120,21 +120,21 @@ pub struct OfflineCategorizationResult { /// Categorizes a list of filenames using extension-based rules. /// Returns categorized files and a list of skipped filenames. -pub fn categorize_files_offline(filenames: &[String]) -> OfflineCategorizationResult { +pub fn categorize_files_offline(filenames: Vec) -> OfflineCategorizationResult { let mut files = Vec::with_capacity(filenames.len()); let mut skipped = Vec::new(); for filename in filenames { - match categorize_by_extension(filename) { + match categorize_by_extension(&filename) { Some(category) => { files.push(FileCategory { - filename: filename.clone(), + filename, category: category.to_string(), sub_category: String::new(), }); } None => { - skipped.push(filename.clone()); + skipped.push(filename); } } } @@ -187,7 +187,7 @@ mod tests { "file.xyz".to_string(), ]; - let result = categorize_files_offline(&filenames); + let result = categorize_files_offline(filenames); assert_eq!(result.plan.files.len(), 2); assert_eq!(result.skipped.len(), 2); diff --git a/src/gemini/client.rs b/src/gemini/client.rs index ff8d751..3ddbf36 100644 --- a/src/gemini/client.rs +++ b/src/gemini/client.rs @@ -41,8 +41,12 @@ impl GeminiClient { } } - pub fn new(api_key: String, categories: Vec) -> Self { - Self::with_model(api_key, DEFAULT_MODEL.to_string(), categories) + pub fn new(api_key: &str, categories: &[String]) -> Self { + Self::with_model( + api_key.to_string(), + DEFAULT_MODEL.to_string(), + categories.to_vec(), + ) } pub fn with_model(api_key: String, model: String, categories: Vec) -> Self { @@ -102,8 +106,7 @@ impl GeminiClient { return Ok(cached_response.clone()); } - let prompt = - PromptBuilder::new(filenames.clone()).build_categorization_prompt(&self.categories); + let prompt = PromptBuilder::new(&filenames).build_categorization_prompt(&self.categories); let request_body = self.build_categorization_request(&prompt); let res = self.send_request_with_retry(&url, &request_body).await?; diff --git a/src/gemini/prompt.rs b/src/gemini/prompt.rs index d279741..1afc3da 100644 --- a/src/gemini/prompt.rs +++ b/src/gemini/prompt.rs @@ -23,7 +23,7 @@ pub struct PromptBuilder { } impl PromptBuilder { - pub fn new(file_list: Vec) -> Self { + pub fn new(file_list: &[String]) -> Self { Self { file_list: file_list.join(", "), } diff --git a/src/settings/prompt.rs b/src/settings/prompt.rs index d349300..295c65f 100644 --- a/src/settings/prompt.rs +++ b/src/settings/prompt.rs @@ -70,14 +70,17 @@ impl Prompter { pub fn prompt_download_folder() -> Result> { let default_path = Self::get_default_downloads_folder(); - let default_display = default_path.to_string_lossy(); + let default_display = &default_path.to_string_lossy(); println!(); println!( "Enter path to folder to organize (e.g., {}):", - default_display.yellow() + &default_display.yellow() + ); + println!( + "Or press Enter to use default: {}", + &default_display.green() ); - println!("Or press Enter to use default: {}", default_display.green()); println!("Folder path: "); let mut attempts = 0;