refactor: cleanup code (#52)

* refactor: cleanup code

* refactor: cleanup code

* refactor: cleanup code

* feat: allow args

---------

Co-authored-by: mike <mike@smartive.ch>

src/lib.rs

@@ -1,4 +1,4 @@
-//! # Simple program to read swiss invoices QR codes as pdf or png files and the relevant data
+//! # Simple program to read Swiss invoice QR codes from PDF or PNG files and extract relevant data
 //!
 //! This program reads QR codes from Swiss invoices and outputs the relevant data as JSON.
 //!
@@ -9,56 +9,56 @@ mod pdf_converter;
 mod qr_parser;
 
 use crate::models::qr_data::QRData;
-use image;
+use image::DynamicImage;
 use rayon::prelude::*;
 use rqrr::PreparedImage;
+use std::path::Path;
 use tempfile::tempdir;
 
-pub fn get_qr_bill_data(file_path: String, fail_on_error: bool) -> Vec<QRData> {
-    let tmp_dir = tempdir().expect("Error creating temporary directory");
-
-    let images = match file_path.to_lowercase().as_str() {
-        input if input.ends_with(".pdf") => {
-            pdf_converter::convert_to_png(&file_path, &tmp_dir.path())
-        }
+pub fn get_qr_bill_data(file_path: &str, fail_on_error: bool) -> Vec<QRData> {
+    let tmp_dir = tempdir().expect("Failed to create temporary directory");
+
+    let images = load_images(file_path, tmp_dir.path());
+
+    let qr_data_results: Vec<_> = images
+        .into_par_iter()
+        .flat_map(|img| extract_qr_data(&img))
+        .collect();
+
+    handle_errors(&qr_data_results, fail_on_error);
+
+    qr_data_results.into_iter().filter_map(Result::ok).collect()
+}
+
+fn load_images(file_path: &str, tmp_dir_path: &Path) -> Vec<DynamicImage> {
+    match file_path.to_lowercase().as_str() {
+        input if input.ends_with(".pdf") => pdf_converter::convert_to_png(file_path, tmp_dir_path),
         input if input.ends_with(".png") || input.ends_with(".jpg") || input.ends_with(".jpeg") => {
-            vec![image::open(&file_path).expect("Error loading image")]
+            vec![image::open(file_path).expect("Failed to load image")]
         }
         _ => panic!("Unsupported file format"),
-    };
+    }
+}
 
-    let all_qr_codes: Vec<_> = images
+fn extract_qr_data(img: &DynamicImage) -> Vec<Result<QRData, String>> {
+    PreparedImage::prepare(img.to_luma8())
+        .detect_grids()
         .into_par_iter()
-        .map(|img| {
-            let mut img = PreparedImage::prepare(img.to_luma8());
-            img.detect_grids()
-                .into_par_iter()
-                .filter_map(|result| result.decode().ok())
-                .map(|(_, content)| qr_parser::get_qr_code_data(&content))
-                .collect::<Vec<_>>()
-        })
-        .flatten()
-        .collect();
+        .filter_map(|grid| grid.decode().ok())
+        .map(|(_, content)| qr_parser::get_qr_code_data(&content))
+        .collect()
+}
 
-    // check if there were any errors
-    if fail_on_error && all_qr_codes.iter().any(|result| result.is_err()) {
+fn handle_errors(results: &[Result<QRData, String>], fail_on_error: bool) {
+    if fail_on_error && results.iter().any(Result::is_err) {
         eprintln!("Error parsing QR codes");
 
-        // print the errors
-        for result in all_qr_codes {
+        for result in results {
             if let Err(err) = result {
                 eprintln!("{}", err);
             }
         }
 
         std::process::exit(1);
     }
-
-    let all_qr_codes: Vec<_> = all_qr_codes
-        .into_iter()
-        .filter(|result| result.is_ok())
-        .map(|result| result.unwrap())
-        .collect();
-
-    return all_qr_codes;
 }

src/main.rs

@@ -11,13 +11,14 @@ use swiss_qr_bill_decoder::get_qr_bill_data;
 
 fn main() {
     let args = args::Args::parse();
-    let all_qr_codes: Vec<_> = get_qr_bill_data(args.input, args.fail_on_error);
+    let all_qr_codes: Vec<_> = get_qr_bill_data(args.input.as_ref(), args.fail_on_error);
 
-    // send the QR code data to stdout
-    if args.pretty {
-        serde_json::to_writer_pretty(std::io::stdout(), &all_qr_codes)
+    // Serialize QR code data to stdout
+    let writer = if args.pretty {
+        serde_json::to_writer_pretty
     } else {
-        serde_json::to_writer(std::io::stdout(), &all_qr_codes)
-    }
-    .expect("Error writing JSON");
+        serde_json::to_writer
+    };
+
+    writer(std::io::stdout(), &all_qr_codes).expect("Error writing JSON");
 }

src/models/qr_data.rs

@@ -14,6 +14,8 @@ pub struct QRData {
 }
 
 impl QRData {
+
+    #[allow(clippy::too_many_arguments)]
     pub fn new(
         iban: String,
         recipient_address: Address,

src/pdf_converter.rs

@@ -21,7 +21,7 @@ where
     Q: AsRef<Path>,
 {
     Command::new("gs")
-        .args(&[
+        .args([
             "-q",
             "-dBATCH",
             "-dSAFER",

src/qr_parser.rs

@@ -3,98 +3,60 @@ use crate::models::qr_data::QRData;
 use std::str::Lines;
 
 /// Get the QR code data from a String according to the Swiss QR bill standard
-pub fn get_qr_code_data(text: &String) -> Result<QRData, String> {
+pub fn get_qr_code_data(text: &str) -> Result<QRData, String> {
     let mut lines = text.lines();
 
-    if lines.next() != Some("SPC") {
-        return Err("First line is not 'SPC'".to_string());
-    }
-
-    if lines.next() != Some("0200") {
-        return Err("Only version 0200 is supported".to_string());
-    }
-
-    if lines.next() != Some("1") {
-        return Err("Only coding type 1 (UTF-8) is supported".to_string());
-    }
+    check_line(&mut lines, "SPC", "First line is not 'SPC'")?;
+    check_line(&mut lines, "0200", "Only version 0200 is supported")?;
+    check_line(&mut lines, "1", "Only coding type 1 (UTF-8) is supported")?;
 
     let iban = match lines.next() {
-        Some(iban) if iban.is_empty() => return Err("Missing IBAN".to_string()),
+        Some("") => return Err("Missing IBAN".to_string()),
         Some(iban) if iban.starts_with("CH") || iban.starts_with("LI") => iban.to_string(),
         _ => return Err("Only CH and LI IBANs are supported".to_string()),
     };
 
-    let address_type = match lines.next() {
-        Some(address_type) if address_type.is_empty() => {
-            return Err("Recipient address type is empty".to_string())
-        }
-        Some(address_type) => address_type,
-        _ => return Err("Missing recipient address type".to_string()),
-    };
-
+    let address_type = lines.next().ok_or("Missing recipient address type")?;
     let recipient_address = to_address(&mut lines, address_type)?;
 
     skip_lines(&mut lines, 7);
 
-    let amount = match lines.next() {
-        Some(amount) if amount.is_empty() => None,
-        Some(amount) => Some(amount.trim().to_string()),
-        _ => return Err("Missing amount".to_string()),
-    };
-
+    let amount = lines.next().filter(|s| !s.is_empty()).map(str::to_string);
     let currency = match lines.next() {
-        Some(currency) if currency.is_empty() => return Err("Missing currency".to_string()),
-        Some(currency) if currency.eq("CHF") || currency.eq("EUR") => currency.to_string(),
+        Some("") => return Err("Missing currency".to_string()),
+        Some(currency) if currency == "CHF" || currency == "EUR" => currency.to_string(),
         _ => return Err("Only CHF and EUR currencies are supported".to_string()),
     };
 
-    let address_type = match lines.next() {
-        Some(address_type) if address_type.is_empty() => None,
-        Some(address_type) => Some(address_type),
-        _ => return Err("Missing address type".to_string()),
-    };
-
-    let sender_address = if address_type.is_some() {
-        Some(to_address(&mut lines, address_type.unwrap())?)
+    let address_type = lines.next().filter(|s| !s.is_empty());
+    let sender_address = if let Some(address_type) = address_type {
+        Some(to_address(&mut lines, address_type)?)
     } else {
         skip_lines(&mut lines, 6);
         None
     };
 
     let reference_type = match lines.next() {
-        Some(reference_type) if reference_type.is_empty() => {
-            return Err("Missing reference type".to_string())
-        }
-        Some(reference_type)
-            if reference_type.eq("NON")
-                || reference_type.eq("QRR")
-                || reference_type.eq("SCOR") =>
-        {
+        Some("") => return Err("Missing reference type".to_string()),
+        Some(reference_type) if ["NON", "QRR", "SCOR"].contains(&reference_type) => {
             reference_type.to_string()
         }
-        _ => return Err("Only reference types NON, QRR and SCOR are supported".to_string()),
+        _ => return Err("Only reference types NON, QRR, and SCOR are supported".to_string()),
     };
 
     let reference = match lines.next() {
         Some(reference)
-            if reference.is_empty() && (reference_type.eq("QRR") || reference_type.eq("SCOR")) =>
+            if reference.is_empty() && ["QRR", "SCOR"].contains(&reference_type.as_str()) =>
         {
-            return Err("Reference is empty".to_string())
+            return Err("Reference is empty".to_string());
         }
-        Some(reference) if reference.is_empty() => None,
+        Some("") => None,
         Some(reference) => Some(reference.trim().to_string()),
         _ => return Err("Missing reference".to_string()),
     };
 
-    let message = match lines.next() {
-        Some(message) if message.is_empty() => None,
-        Some(message) => Some(message.trim().to_string()),
-        _ => return Err("Missing message".to_string()),
-    };
-
-    if lines.next() != Some("EPD") {
-        return Err("Missing trailing 'EPD'".to_string());
-    }
+    let message = lines.next().filter(|s| !s.is_empty()).map(str::to_string);
+    check_line(&mut lines, "EPD", "Missing trailing 'EPD'")?;
 
     Ok(QRData::new(
         iban,
@@ -108,60 +70,55 @@ pub fn get_qr_code_data(text: &String) -> Result<QRData, String> {
     ))
 }
 
-fn skip_lines(lines: &mut Lines, skip_lines: i32) {
-    for _ in 0..skip_lines {
+fn check_line(lines: &mut Lines, expected: &str, error_msg: &str) -> Result<(), String> {
+    if lines.next() != Some(expected) {
+        return Err(error_msg.to_string());
+    }
+    Ok(())
+}
+
+fn skip_lines(lines: &mut Lines, skip_count: i32) {
+    for _ in 0..skip_count {
         let _ = lines.next();
     }
 }
 
 fn to_address(lines: &mut Lines, address_type: &str) -> Result<Address, String> {
     let address_type = match address_type {
-        address_type if address_type.is_empty() => return Err("Address type is empty".to_string()),
-        address_type if !address_type.eq("K") && !address_type.eq("S") => {
+        "" => return Err("Address type is empty".to_string()),
+        address_type if !["K", "S"].contains(&address_type) => {
             return Err("Only address types K and S are supported".to_string())
         }
         address_type => address_type.to_string(),
     };
 
-    let name = match lines.next() {
-        None => return Err("Missing name".to_string()),
-        Some(name) if name.is_empty() => return Err("Recipient name is empty".to_string()),
-        Some(name) => name.to_string(),
-    };
-
-    let street_or_address_line_1 = match lines.next() {
-        None => return Err("Missing street or address line 1".to_string()),
-        Some(street_or_address_line_1) => street_or_address_line_1.to_string(),
-    };
-
-    let building_number_or_address_line_2 = match lines.next() {
-        None => return Err("Missing building number or address line 2".to_string()),
-        Some(building_number_or_address_line_2) => building_number_or_address_line_2.to_string(),
-    };
-
-    let postal_code = match lines.next() {
-        None => return Err("Missing postal code".to_string()),
-        Some(postal_code) => postal_code.to_string(),
-    };
-
-    let town = match lines.next() {
-        None => return Err("Missing town".to_string()),
-        Some(town) => town.to_string(),
-    };
-
-    let country = match lines.next() {
-        None => return Err("Missing country".to_string()),
-        Some(country) => country.to_string(),
-    };
-
-    let address_line_1 = if address_type.eq("K") {
-        street_or_address_line_1.to_string()
+    let name = lines.next().ok_or("Missing name".to_string())?.to_string();
+    let street_or_address_line_1 = lines
+        .next()
+        .ok_or("Missing street or address line 1".to_string())?
+        .to_string();
+    let building_number_or_address_line_2 = lines
+        .next()
+        .ok_or("Missing building number or address line 2".to_string())?
+        .to_string();
+    let postal_code = lines
+        .next()
+        .ok_or("Missing postal code".to_string())?
+        .to_string();
+    let town = lines.next().ok_or("Missing town".to_string())?.to_string();
+    let country = lines
+        .next()
+        .ok_or("Missing country".to_string())?
+        .to_string();
+
+    let address_line_1 = if address_type == "K" {
+        street_or_address_line_1.clone()
     } else {
         format!("{street_or_address_line_1} {building_number_or_address_line_2}")
     };
 
-    let address_line_2 = if address_type.eq("K") {
-        building_number_or_address_line_2.to_string()
+    let address_line_2 = if address_type == "K" {
+        building_number_or_address_line_2.clone()
     } else {
         format!("{postal_code} {town}")
     };