diff --git a/exercises/practice/robot-name/.docs/instructions.append.md b/exercises/practice/robot-name/.docs/instructions.append.md deleted file mode 100644 index 48930b13d..000000000 --- a/exercises/practice/robot-name/.docs/instructions.append.md +++ /dev/null @@ -1,9 +0,0 @@ -# Instructions append - -## Global Mutable State - -The way the tests are setup, you will be forced to use global mutable state. -This is generally frowned-upon and considered unidiomatic in Rust. -However, it's a delibarate choice for the purpose of learning here. - -Can you think of a better API design that doesn't use global mutable state? diff --git a/exercises/practice/robot-name/.meta/Cargo-example.toml b/exercises/practice/robot-name/.meta/Cargo-example.toml deleted file mode 100644 index bceec03b5..000000000 --- a/exercises/practice/robot-name/.meta/Cargo-example.toml +++ /dev/null @@ -1,10 +0,0 @@ -[package] -name = "robot_name" -version = "0.1.0" -edition = "2024" - -# Not all libraries from crates.io are available in Exercism's test runner. -# The full list of available libraries is here: -# https://github.com/exercism/rust-test-runner/blob/main/local-registry/Cargo.toml -[dependencies] -rand = "0.3.12" diff --git a/exercises/practice/robot-name/.meta/config.json b/exercises/practice/robot-name/.meta/config.json index c5d2fa26d..2a1722971 100644 --- a/exercises/practice/robot-name/.meta/config.json +++ b/exercises/practice/robot-name/.meta/config.json @@ -22,6 +22,7 @@ "pminten", "razielgn", "rofrol", + "senekor", "spazm", "stringparser", "workingjubilee", diff --git a/exercises/practice/robot-name/.meta/example.rs b/exercises/practice/robot-name/.meta/example.rs index 487312a77..73e783d12 100644 --- a/exercises/practice/robot-name/.meta/example.rs +++ b/exercises/practice/robot-name/.meta/example.rs @@ -1,52 +1,64 @@ use std::{ collections::HashSet, - sync::{LazyLock, Mutex}, + sync::{Arc, Mutex}, }; -use rand::{Rng, thread_rng}; +use rand::Rng; -static NAMES: LazyLock>> = LazyLock::new(|| Mutex::new(HashSet::new())); - -pub struct Robot { - name: String, -} - -fn generate_name() -> String { +fn generate_name(rng: &mut R, used_names: &mut HashSet) -> String { loop { let mut s = String::with_capacity(5); - static LETTERS: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ"; - static NUMBERS: &[u8] = b"0123456789"; for _ in 0..2 { - s.push(*thread_rng().choose(LETTERS).unwrap() as char); + s.push(rng.random_range('A'..='Z')); } for _ in 0..3 { - s.push(*thread_rng().choose(NUMBERS).unwrap() as char); + s.push(rng.random_range('0'..='9')); } - - if NAMES.lock().unwrap().insert(s.clone()) { + if used_names.insert(s.clone()) { return s; } } } -impl Robot { - pub fn new() -> Robot { - Robot { - name: generate_name(), +/// A `RobotFactory` is responsible for ensuring that all robots produced by +/// it have a unique name. Robots from different factories can have the same +/// name. +pub struct RobotFactory { + used_names: Arc>>, +} + +pub struct Robot { + used_names: Arc>>, + name: String, +} + +impl RobotFactory { + pub fn new() -> Self { + Self { + used_names: Arc::new(Mutex::new(HashSet::new())), } } - pub fn name(&self) -> &str { - &self.name[..] + pub fn new_robot(&mut self, rng: &mut R) -> Robot { + let mut guard = self.used_names.lock().unwrap(); + Robot { + used_names: Arc::clone(&self.used_names), + name: generate_name(rng, &mut guard), + } } +} - pub fn reset_name(&mut self) { - self.name = generate_name(); +impl Robot { + pub fn name(&self) -> &str { + &self.name } -} -impl Default for Robot { - fn default() -> Self { - Self::new() + // When a robot is reset, its factory must still ensure that there are no + // name conflicts with other robots from the same factory. + pub fn reset(&mut self, rng: &mut R) { + let mut used_names = self.used_names.lock().unwrap(); + let new_name = generate_name(rng, &mut used_names); + used_names.remove(&self.name); + self.name = new_name } } diff --git a/exercises/practice/robot-name/Cargo.toml b/exercises/practice/robot-name/Cargo.toml index d584df2f0..efa2a4786 100644 --- a/exercises/practice/robot-name/Cargo.toml +++ b/exercises/practice/robot-name/Cargo.toml @@ -7,6 +7,7 @@ edition = "2024" # The full list of available libraries is here: # https://github.com/exercism/rust-test-runner/blob/main/local-registry/Cargo.toml [dependencies] +rand = "0.9" [lints.clippy] new_without_default = "allow" diff --git a/exercises/practice/robot-name/src/lib.rs b/exercises/practice/robot-name/src/lib.rs index 37bf158ed..6db419e9b 100644 --- a/exercises/practice/robot-name/src/lib.rs +++ b/exercises/practice/robot-name/src/lib.rs @@ -1,15 +1,28 @@ +use rand::Rng; + +/// A `RobotFactory` is responsible for ensuring that all robots produced by +/// it have a unique name. Robots from different factories can have the same +/// name. +pub struct RobotFactory; + pub struct Robot; -impl Robot { +impl RobotFactory { pub fn new() -> Self { - todo!("Construct a new Robot struct."); + todo!("Create a new robot factory") } + pub fn new_robot(&mut self, _rng: &mut R) -> Robot { + todo!("Create a new robot with a unique name") + } +} + +impl Robot { pub fn name(&self) -> &str { - todo!("Return the reference to the robot's name."); + todo!("Return a reference to the robot's name"); } - pub fn reset_name(&mut self) { - todo!("Assign a new unique name to the robot."); + pub fn reset(&mut self, _rng: &mut R) { + todo!("Assign a new unique name to the robot"); } } diff --git a/exercises/practice/robot-name/tests/robot_name.rs b/exercises/practice/robot-name/tests/robot_name.rs index abfeadd3f..545d79796 100644 --- a/exercises/practice/robot-name/tests/robot_name.rs +++ b/exercises/practice/robot-name/tests/robot_name.rs @@ -1,4 +1,10 @@ -use robot_name as robot; +use rand::SeedableRng as _; +use rand::rngs::SmallRng; +use robot_name::*; + +fn deterministic_rng() -> SmallRng { + SmallRng::seed_from_u64(0) +} fn assert_name_matches_pattern(n: &str) { assert!(n.len() == 5, "name is exactly 5 characters long"); @@ -14,46 +20,86 @@ fn assert_name_matches_pattern(n: &str) { #[test] fn name_should_match_expected_pattern() { - let r = robot::Robot::new(); - assert_name_matches_pattern(r.name()); + let mut rng = deterministic_rng(); + let mut factory = RobotFactory::new(); + let robot = factory.new_robot(&mut rng); + assert_name_matches_pattern(robot.name()); } #[test] #[ignore] -fn different_robots_have_different_names() { - let r1 = robot::Robot::new(); - let r2 = robot::Robot::new(); - assert_ne!(r1.name(), r2.name(), "Robot names should be different"); +fn robot_name_depends_on_rng() { + let mut rng = deterministic_rng(); + let robot_1 = RobotFactory::new().new_robot(&mut rng); + let robot_2 = RobotFactory::new().new_robot(&mut rng); + assert_ne!(robot_1.name(), robot_2.name()); } #[test] #[ignore] -fn many_different_robots_have_different_names() { - use std::collections::HashSet; - - // In 3,529 random robot names, there is ~99.99% chance of a name collision - let vec: Vec<_> = (0..3529).map(|_| robot::Robot::new()).collect(); - let set: HashSet<_> = vec.iter().map(|robot| robot.name()).collect(); +fn robot_name_only_depends_on_rng() { + let robot_1 = RobotFactory::new().new_robot(&mut deterministic_rng()); + let robot_2 = RobotFactory::new().new_robot(&mut deterministic_rng()); + assert_eq!(robot_1.name(), robot_2.name()); +} - let number_of_collisions = vec.len() - set.len(); - assert_eq!(number_of_collisions, 0); +#[test] +#[ignore] +fn factory_prevents_name_collisions() { + let mut factory = RobotFactory::new(); + let robot_1 = factory.new_robot(&mut deterministic_rng()); + let robot_2 = factory.new_robot(&mut deterministic_rng()); + assert_ne!(robot_1.name(), robot_2.name()); } #[test] #[ignore] fn new_name_should_match_expected_pattern() { - let mut r = robot::Robot::new(); - assert_name_matches_pattern(r.name()); - r.reset_name(); - assert_name_matches_pattern(r.name()); + let mut rng = deterministic_rng(); + let mut factory = RobotFactory::new(); + let mut robot = factory.new_robot(&mut rng); + assert_name_matches_pattern(robot.name()); + robot.reset(&mut rng); + assert_name_matches_pattern(robot.name()); } #[test] #[ignore] fn new_name_is_different_from_old_name() { - let mut r = robot::Robot::new(); - let n1 = r.name().to_string(); - r.reset_name(); - let n2 = r.name().to_string(); - assert_ne!(n1, n2, "Robot name should change when reset"); + let mut rng = deterministic_rng(); + let mut factory = RobotFactory::new(); + let mut robot = factory.new_robot(&mut rng); + let name_1 = robot.name().to_string(); + robot.reset(&mut rng); + let name_2 = robot.name().to_string(); + assert_ne!(name_1, name_2, "Robot name should change when reset"); +} + +#[test] +#[ignore] +fn factory_prevents_name_collision_despite_reset() { + let mut factory = RobotFactory::new(); + + let mut rng = deterministic_rng(); + let mut robot_1 = factory.new_robot(&mut rng); + robot_1.reset(&mut rng); + + let mut rng = deterministic_rng(); + let mut robot_2 = factory.new_robot(&mut rng); + robot_2.reset(&mut rng); + + assert_ne!(robot_1.name(), robot_2.name()); +} + +#[test] +#[ignore] +fn old_name_becomes_available_after_reset() { + let mut rng = deterministic_rng(); + let mut factory = RobotFactory::new(); + let mut robot = factory.new_robot(&mut rng); + let first_name = robot.name().to_string(); + robot.reset(&mut rng); // cause first name to become available again + let mut rng = deterministic_rng(); // reset rng + let second_robot = factory.new_robot(&mut rng); + assert_eq!(second_robot.name(), first_name); } diff --git a/rust-tooling/ci-tests/tests/no_underscore_prefixed_idents.rs b/rust-tooling/ci-tests/tests/no_underscore_prefixed_idents.rs index 7fa88b88a..57f58dcf8 100644 --- a/rust-tooling/ci-tests/tests/no_underscore_prefixed_idents.rs +++ b/rust-tooling/ci-tests/tests/no_underscore_prefixed_idents.rs @@ -56,6 +56,7 @@ static EXCEPTIONS: &[&str] = &[ "roman-numerals", // std::fmt::Formatter is not Debug "simple-linked-list", // has generics "sublist", // has generics + "robot-name", // has generics ]; fn line_is_not_a_comment(line: &&str) -> bool {