Refactor command.rs

src/command.rs

@@ -95,106 +95,7 @@ impl<'a> Command<'a> {
     }
 }
 
-/// Finds all the strings that appear multiple times in the input iterator, returning them in
+/// A collection of commands that should be benchmarked
-/// sorted order. If no string appears more than once, the result is an empty vector.
-fn find_duplicates<'a, I: IntoIterator<Item = &'a str>>(i: I) -> Vec<&'a str> {
-    let mut counts = BTreeMap::<&'a str, usize>::new();
-    for s in i {
-        *counts.entry(s).or_default() += 1;
-    }
-    counts
-        .into_iter()
-        .filter_map(|(k, n)| if n > 1 { Some(k) } else { None })
-        .collect()
-}
-
-fn build_parameterized_commands<'a, T: Numeric>(
-    param_min: T,
-    param_max: T,
-    step: T,
-    command_names: Vec<&'a str>,
-    command_strings: Vec<&'a str>,
-    param_name: &'a str,
-) -> Result<Vec<Command<'a>>, ParameterScanError> {
-    let param_range = RangeStep::new(param_min, param_max, step)?;
-    let param_count = param_range.size_hint().1.unwrap();
-    let command_name_count = command_names.len();
-
-    // `--command-name` should appear exactly once or exactly B times,
-    // where B is the total number of benchmarks.
-    if command_name_count > 1 && command_name_count != param_count {
-        return Err(ParameterScanError::UnexpectedCommandNameCount(
-            command_name_count,
-            param_count,
-        ));
-    }
-
-    let mut i = 0;
-    let mut commands = vec![];
-    for value in param_range {
-        for cmd in &command_strings {
-            let name = command_names
-                .get(i)
-                .or_else(|| command_names.get(0))
-                .copied();
-            commands.push(Command::new_parametrized(
-                name,
-                cmd,
-                vec![(param_name, ParameterValue::Numeric(value.into()))],
-            ));
-            i += 1;
-        }
-    }
-    Ok(commands)
-}
-
-fn get_parameterized_commands<'a>(
-    command_names: Option<Values<'a>>,
-    command_strings: Values<'a>,
-    mut vals: clap::Values<'a>,
-    step: Option<&str>,
-) -> Result<Vec<Command<'a>>, ParameterScanError> {
-    let command_names = command_names.map_or(vec![], |names| names.collect::<Vec<&str>>());
-    let command_strings = command_strings.collect::<Vec<&str>>();
-    let param_name = vals.next().unwrap();
-    let param_min = vals.next().unwrap();
-    let param_max = vals.next().unwrap();
-
-    // attempt to parse as integers
-    if let (Ok(param_min), Ok(param_max), Ok(step)) = (
-        param_min.parse::<i32>(),
-        param_max.parse::<i32>(),
-        step.unwrap_or("1").parse::<i32>(),
-    ) {
-        return build_parameterized_commands(
-            param_min,
-            param_max,
-            step,
-            command_names,
-            command_strings,
-            param_name,
-        );
-    }
-
-    // try parsing them as decimals
-    let param_min = Decimal::from_str(param_min)?;
-    let param_max = Decimal::from_str(param_max)?;
-
-    if step.is_none() {
-        return Err(ParameterScanError::StepRequired);
-    }
-
-    let step = Decimal::from_str(step.unwrap())?;
-    build_parameterized_commands(
-        param_min,
-        param_max,
-        step,
-        command_names,
-        command_strings,
-        param_name,
-    )
-}
-
 pub struct Commands<'a>(Vec<Command<'a>>);
 
 impl<'a> Commands<'a> {
@@ -204,7 +105,7 @@ impl<'a> Commands<'a> {
 
         if let Some(args) = matches.values_of("parameter-scan") {
             let step_size = matches.value_of("parameter-step-size");
-            Ok(Self(get_parameterized_commands(
+            Ok(Self(Self::get_parameter_scan_commands(
                 command_names,
                 command_strings,
                 args,
@@ -224,7 +125,7 @@ impl<'a> Commands<'a> {
                 .collect();
             {
                 let duplicates =
-                    find_duplicates(param_names_and_values.iter().map(|(name, _)| *name));
+                    Self::find_duplicates(param_names_and_values.iter().map(|(name, _)| *name));
                 if !duplicates.is_empty() {
                     bail!("Duplicate parameter names: {}", &duplicates.join(", "));
                 }
@@ -311,6 +212,106 @@ impl<'a> Commands<'a> {
     pub fn len(&self) -> usize {
         self.0.len()
     }
+
+    /// Finds all the strings that appear multiple times in the input iterator, returning them in
+    /// sorted order. If no string appears more than once, the result is an empty vector.
+    fn find_duplicates<'b, I: IntoIterator<Item = &'b str>>(i: I) -> Vec<&'b str> {
+        let mut counts = BTreeMap::<&'b str, usize>::new();
+        for s in i {
+            *counts.entry(s).or_default() += 1;
+        }
+        counts
+            .into_iter()
+            .filter_map(|(k, n)| if n > 1 { Some(k) } else { None })
+            .collect()
+    }
+
+    fn build_parameter_scan_commands<'b, T: Numeric>(
+        param_name: &'b str,
+        param_min: T,
+        param_max: T,
+        step: T,
+        command_names: Vec<&'b str>,
+        command_strings: Vec<&'b str>,
+    ) -> Result<Vec<Command<'b>>, ParameterScanError> {
+        let param_range = RangeStep::new(param_min, param_max, step)?;
+        let param_count = param_range.size_hint().1.unwrap();
+        let command_name_count = command_names.len();
+
+        // `--command-name` should appear exactly once or exactly B times,
+        // where B is the total number of benchmarks.
+        if command_name_count > 1 && command_name_count != param_count {
+            return Err(ParameterScanError::UnexpectedCommandNameCount(
+                command_name_count,
+                param_count,
+            ));
+        }
+
+        let mut i = 0;
+        let mut commands = vec![];
+        for value in param_range {
+            for cmd in &command_strings {
+                let name = command_names
+                    .get(i)
+                    .or_else(|| command_names.get(0))
+                    .copied();
+                commands.push(Command::new_parametrized(
+                    name,
+                    cmd,
+                    vec![(param_name, ParameterValue::Numeric(value.into()))],
+                ));
+                i += 1;
+            }
+        }
+        Ok(commands)
+    }
+
+    fn get_parameter_scan_commands<'b>(
+        command_names: Option<Values<'b>>,
+        command_strings: Values<'b>,
+        mut vals: clap::Values<'b>,
+        step: Option<&str>,
+    ) -> Result<Vec<Command<'b>>, ParameterScanError> {
+        let command_names = command_names.map_or(vec![], |names| names.collect::<Vec<&str>>());
+        let command_strings = command_strings.collect::<Vec<&str>>();
+        let param_name = vals.next().unwrap();
+        let param_min = vals.next().unwrap();
+        let param_max = vals.next().unwrap();
+
+        // attempt to parse as integers
+        if let (Ok(param_min), Ok(param_max), Ok(step)) = (
+            param_min.parse::<i32>(),
+            param_max.parse::<i32>(),
+            step.unwrap_or("1").parse::<i32>(),
+        ) {
+            return Self::build_parameter_scan_commands(
+                param_name,
+                param_min,
+                param_max,
+                step,
+                command_names,
+                command_strings,
+            );
+        }
+
+        // try parsing them as decimals
+        let param_min = Decimal::from_str(param_min)?;
+        let param_max = Decimal::from_str(param_max)?;
+
+        if step.is_none() {
+            return Err(ParameterScanError::StepRequired);
+        }
+
+        let step = Decimal::from_str(step.unwrap())?;
+        Self::build_parameter_scan_commands(
+            param_name,
+            param_min,
+            param_max,
+            step,
+            command_names,
+            command_strings,
+        )
+    }
 }
 
 #[test]
@@ -405,7 +406,7 @@ fn test_build_parameter_list_commands() {
 }
 
 #[test]
-fn test_build_parameter_range_commands() {
+fn test_build_parameter_scan_commands() {
     use crate::app::get_cli_arguments;
     let matches = get_cli_arguments(vec![
         "hyperfine",
@@ -428,27 +429,34 @@ fn test_build_parameter_range_commands() {
 }
 
 #[test]
-fn test_get_parameterized_commands_int() {
+fn test_parameter_scan_commands_int() {
-    let commands =
+    let commands = Commands::build_parameter_scan_commands(
-        build_parameterized_commands(1i32, 7i32, 3i32, vec![], vec!["echo {val}"], "val").unwrap();
+        "val",
+        1i32,
+        7i32,
+        3i32,
+        vec![],
+        vec!["echo {val}"],
+    )
+    .unwrap();
     assert_eq!(commands.len(), 3);
     assert_eq!(commands[2].get_name(), "echo 7");
     assert_eq!(commands[2].get_shell_command(), "echo 7");
 }
 
 #[test]
-fn test_get_parameterized_commands_decimal() {
+fn test_parameter_scan_commands_decimal() {
     let param_min = Decimal::from_str("0").unwrap();
     let param_max = Decimal::from_str("1").unwrap();
     let step = Decimal::from_str("0.33").unwrap();
 
-    let commands = build_parameterized_commands(
+    let commands = Commands::build_parameter_scan_commands(
+        "val",
         param_min,
         param_max,
         step,
         vec![],
         vec!["echo {val}"],
-        "val",
     )
     .unwrap();
     assert_eq!(commands.len(), 4);
@@ -457,14 +465,14 @@ fn test_get_parameterized_commands_decimal() {
 }
 
 #[test]
-fn test_get_parameterized_command_names() {
+fn test_parameterr_scan_commands_names() {
-    let commands = build_parameterized_commands(
+    let commands = Commands::build_parameter_scan_commands(
+        "val",
         1i32,
         3i32,
         1i32,
         vec!["name-{val}"],
         vec!["echo {val}"],
-        "val",
     )
     .unwrap();
     assert_eq!(commands.len(), 3);
@@ -477,13 +485,13 @@ fn test_get_parameterized_command_names() {
 
 #[test]
 fn test_get_specified_command_names() {
-    let commands = build_parameterized_commands(
+    let commands = Commands::build_parameter_scan_commands(
+        "val",
         1i32,
         3i32,
         1i32,
         vec!["name-a", "name-b", "name-c"],
         vec!["echo {val}"],
-        "val",
     )
     .unwrap();
     assert_eq!(commands.len(), 3);
@@ -496,16 +504,16 @@ fn test_get_specified_command_names() {
 
 #[test]
 fn test_different_command_name_count_with_parameters() {
-    let result = build_parameterized_commands(
+    let result = Commands::build_parameter_scan_commands(
+        "val",
         1i32,
         3i32,
         1i32,
         vec!["name-1", "name-2"],
         vec!["echo {val}"],
-        "val",
-    );
-    assert_eq!(
-        format!("{}", result.unwrap_err()),
-        "'--command-name' has been specified 2 times. It has to appear exactly once, or exactly 3 times (number of benchmarks)"
     );
+    assert!(matches!(
+        result.unwrap_err(),
+        ParameterScanError::UnexpectedCommandNameCount(2, 3)
+    ));
 }