diff --git a/exercises/react/example.rs b/exercises/react/example.rs index f3ac5c60c..096426960 100644 --- a/exercises/react/example.rs +++ b/exercises/react/example.rs @@ -3,6 +3,18 @@ use std::collections::HashMap; pub type CellID = usize; pub type CallbackID = usize; +#[derive(Debug, PartialEq)] +pub enum SetValueError { + NonexistentCell, + ComputeCell, +} + +#[derive(Debug, PartialEq)] +pub enum RemoveCallbackError { + NonexistentCell, + NonexistentCallback, +} + struct Cell<'a, T: Copy> { value: T, last_value: T, @@ -46,11 +58,11 @@ impl <'a, T: Copy + PartialEq> Reactor<'a, T> { self.cells.len() - 1 } - pub fn create_compute T + 'a>(&mut self, dependencies: &[CellID], compute_func: F) -> Result { + pub fn create_compute T + 'a>(&mut self, dependencies: &[CellID], compute_func: F) -> Result { // Check all dependencies' validity before modifying any of them, // so that we don't perform an incorrect partial write. - if !dependencies.iter().all(|&id| id < self.cells.len()) { - return Err("Nonexistent input"); + if let Some(&invalid) = dependencies.iter().find(|&dep| *dep >= self.cells.len()) { + return Err(invalid); } let new_id = self.cells.len(); for &id in dependencies { @@ -66,16 +78,16 @@ impl <'a, T: Copy + PartialEq> Reactor<'a, T> { self.cells.get(id).map(|c| c.value) } - pub fn set_value(&mut self, id: CellID, new_value: T) -> Result<(), &'static str> { + pub fn set_value(&mut self, id: CellID, new_value: T) -> Result<(), SetValueError> { match self.cells.get_mut(id) { Some(c) => match c.cell_type { CellType::Input => { c.value = new_value; Ok(c.dependents.clone()) }, - CellType::Compute(_, _) => Err("Can't set compute cell value directly"), + CellType::Compute(_, _) => Err(SetValueError::ComputeCell), }, - None => Err("Can't set nonexistent cell"), + None => Err(SetValueError::NonexistentCell), }.map(|deps| { for &d in deps.iter() { self.update_dependent(d); @@ -88,24 +100,21 @@ impl <'a, T: Copy + PartialEq> Reactor<'a, T> { }) } - pub fn add_callback () + 'a>(&mut self, id: CellID, callback: F) -> Result { - match self.cells.get_mut(id) { - Some(c) => { - c.callbacks_issued += 1; - c.callbacks.insert(c.callbacks_issued, Box::new(callback)); - Ok(c.callbacks_issued) - }, - None => Err("Can't add callback to nonexistent cell"), - } + pub fn add_callback () + 'a>(&mut self, id: CellID, callback: F) -> Option { + self.cells.get_mut(id).map(|c| { + c.callbacks_issued += 1; + c.callbacks.insert(c.callbacks_issued, Box::new(callback)); + c.callbacks_issued + }) } - pub fn remove_callback(&mut self, cell: CellID, callback: CallbackID) -> Result<(), &'static str> { + pub fn remove_callback(&mut self, cell: CellID, callback: CallbackID) -> Result<(), RemoveCallbackError> { match self.cells.get_mut(cell) { Some(c) => match c.callbacks.remove(&callback) { Some(_) => Ok(()), - None => Err("Can't remove nonexistent callback"), + None => Err(RemoveCallbackError::NonexistentCallback), }, - None => Err("Can't remove callback from nonexistent cell"), + None => Err(RemoveCallbackError::NonexistentCell), } } diff --git a/exercises/react/src/lib.rs b/exercises/react/src/lib.rs index 4ec956768..600840ad2 100644 --- a/exercises/react/src/lib.rs +++ b/exercises/react/src/lib.rs @@ -5,6 +5,18 @@ pub type CellID = (); pub type CallbackID = (); +#[derive(Debug, PartialEq)] +pub enum SetValueError { + NonexistentCell, + ComputeCell, +} + +#[derive(Debug, PartialEq)] +pub enum RemoveCallbackError { + NonexistentCell, + NonexistentCallback, +} + pub struct Reactor { // Just so that the compiler doesn't complain about an unused type parameter. // You probably want to delete this field. @@ -28,12 +40,14 @@ impl Reactor { // You do not need to reject compute functions that expect more arguments than there are // dependencies (how would you check for this, anyway?). // - // Return an Err (and you can change the error type) if any dependency doesn't exist. + // If any dependency doesn't exist, returns an Err with that nonexistent dependency. + // (If multiple dependencies do not exist, exactly which one is returned is not defined and + // will not be tested) // // Notice that there is no way to *remove* a cell. // This means that you may assume, without checking, that if the dependencies exist at creation // time they will continue to exist as long as the Reactor exists. - pub fn create_compute T>(&mut self, dependencies: &[CellID], compute_func: F) -> Result { + pub fn create_compute T>(&mut self, dependencies: &[CellID], compute_func: F) -> Result { unimplemented!() } @@ -50,20 +64,22 @@ impl Reactor { // Sets the value of the specified input cell. // - // Return an Err (and you can change the error type) if the cell does not exist, or the - // specified cell is a compute cell, since compute cells cannot have their values directly set. + // Returns an Err if either: + // * the cell does not exist + // * the specified cell is a compute cell, since compute cells cannot have their values + // directly set. // // Similarly, you may wonder about `get_mut(&mut self, id: CellID) -> Option<&mut Cell>`, with // a `set_value(&mut self, new_value: T)` method on `Cell`. // // As before, that turned out to add too much extra complexity. - pub fn set_value(&mut self, id: CellID, new_value: T) -> Result<(), ()> { + pub fn set_value(&mut self, id: CellID, new_value: T) -> Result<(), SetValueError> { unimplemented!() } // Adds a callback to the specified compute cell. // - // Return an Err (and you can change the error type) if the cell does not exist. + // Returns the ID of the just-added callback, or None if the cell doesn't exist. // // Callbacks on input cells will not be tested. // @@ -73,17 +89,16 @@ impl Reactor { // * Exactly once if the compute cell's value changed as a result of the set_value call. // The value passed to the callback should be the final value of the compute cell after the // set_value call. - pub fn add_callback ()>(&mut self, id: CellID, callback: F) -> Result { + pub fn add_callback ()>(&mut self, id: CellID, callback: F) -> Option { unimplemented!() } // Removes the specified callback, using an ID returned from add_callback. // - // Return an Err (and you can change the error type) if either the cell or callback - // does not exist. + // Returns an Err if either the cell or callback does not exist. // // A removed callback should no longer be called. - pub fn remove_callback(&mut self, cell: CellID, callback: CallbackID) -> Result<(), ()> { + pub fn remove_callback(&mut self, cell: CellID, callback: CallbackID) -> Result<(), RemoveCallbackError> { unimplemented!() } } diff --git a/exercises/react/tests/react.rs b/exercises/react/tests/react.rs index c4af9c775..95ab71613 100644 --- a/exercises/react/tests/react.rs +++ b/exercises/react/tests/react.rs @@ -23,7 +23,7 @@ fn an_input_cells_value_can_be_set() { fn error_setting_a_nonexistent_input_cell() { let mut dummy_reactor = Reactor::new(); let input = dummy_reactor.create_input(1); - assert!(Reactor::new().set_value(input, 0).is_err()); + assert_eq!(Reactor::new().set_value(input, 0), Err(SetValueError::NonexistentCell)); } #[test] @@ -50,7 +50,7 @@ fn compute_cells_take_inputs_in_the_right_order() { fn error_creating_compute_cell_if_input_doesnt_exist() { let mut dummy_reactor = Reactor::new(); let input = dummy_reactor.create_input(1); - assert!(Reactor::new().create_compute(&[input], |_| 0).is_err()); + assert_eq!(Reactor::new().create_compute(&[input], |_| 0), Err(input)); } #[test] @@ -61,7 +61,7 @@ fn do_not_break_cell_if_creating_compute_cell_with_valid_and_invalid_input() { let dummy_cell = dummy_reactor.create_input(2); let mut reactor = Reactor::new(); let input = reactor.create_input(1); - assert!(reactor.create_compute(&[input, dummy_cell], |_| 0).is_err()); + assert_eq!(reactor.create_compute(&[input, dummy_cell], |_| 0), Err(dummy_cell)); assert!(reactor.set_value(input, 5).is_ok()); assert_eq!(reactor.value(input), Some(5)); } @@ -96,7 +96,7 @@ fn error_setting_a_compute_cell() { let mut reactor = Reactor::new(); let input = reactor.create_input(1); let output = reactor.create_compute(&[input], |_| 0).unwrap(); - assert!(reactor.set_value(output, 3).is_err()); + assert_eq!(reactor.set_value(output, 3), Err(SetValueError::ComputeCell)); } /// A CallbackRecorder helps tests whether callbacks get called correctly. @@ -139,7 +139,7 @@ fn compute_cells_fire_callbacks() { let mut reactor = Reactor::new(); let input = reactor.create_input(1); let output = reactor.create_compute(&[input], |v| v[0] + 1).unwrap(); - assert!(reactor.add_callback(output, |v| cb.callback_called(v)).is_ok()); + assert!(reactor.add_callback(output, |v| cb.callback_called(v)).is_some()); assert!(reactor.set_value(input, 3).is_ok()); cb.expect_to_have_been_called_with(4); } @@ -150,7 +150,7 @@ fn error_adding_callback_to_nonexistent_cell() { let mut dummy_reactor = Reactor::new(); let input = dummy_reactor.create_input(1); let output = dummy_reactor.create_compute(&[input], |_| 0).unwrap(); - assert!(Reactor::new().add_callback(output, |_: usize| println!("hi")).is_err()); + assert_eq!(Reactor::new().add_callback(output, |_: usize| println!("hi")), None); } #[test] @@ -160,7 +160,7 @@ fn callbacks_only_fire_on_change() { let mut reactor = Reactor::new(); let input = reactor.create_input(1); let output = reactor.create_compute(&[input], |v| if v[0] < 3 { 111 } else { 222 }).unwrap(); - assert!(reactor.add_callback(output, |v| cb.callback_called(v)).is_ok()); + assert!(reactor.add_callback(output, |v| cb.callback_called(v)).is_some()); assert!(reactor.set_value(input, 2).is_ok()); cb.expect_not_to_have_been_called(); @@ -180,14 +180,14 @@ fn callbacks_can_be_added_and_removed() { let output = reactor.create_compute(&[input], |v| v[0] + 1).unwrap(); let callback = reactor.add_callback(output, |v| cb1.callback_called(v)).unwrap(); - assert!(reactor.add_callback(output, |v| cb2.callback_called(v)).is_ok()); + assert!(reactor.add_callback(output, |v| cb2.callback_called(v)).is_some()); assert!(reactor.set_value(input, 31).is_ok()); cb1.expect_to_have_been_called_with(32); cb2.expect_to_have_been_called_with(32); assert!(reactor.remove_callback(output, callback).is_ok()); - assert!(reactor.add_callback(output, |v| cb3.callback_called(v)).is_ok()); + assert!(reactor.add_callback(output, |v| cb3.callback_called(v)).is_some()); assert!(reactor.set_value(input, 41).is_ok()); cb1.expect_not_to_have_been_called(); @@ -205,11 +205,11 @@ fn removing_a_callback_multiple_times_doesnt_interfere_with_other_callbacks() { let input = reactor.create_input(1); let output = reactor.create_compute(&[input], |v| v[0] + 1).unwrap(); let callback = reactor.add_callback(output, |v| cb1.callback_called(v)).unwrap(); - assert!(reactor.add_callback(output, |v| cb2.callback_called(v)).is_ok()); - // We want the first remove to be Ok, but we don't care about the others. + assert!(reactor.add_callback(output, |v| cb2.callback_called(v)).is_some()); + // We want the first remove to be Ok, but the others should be errors. assert!(reactor.remove_callback(output, callback).is_ok()); for _ in 1..5 { - assert!(reactor.remove_callback(output, callback).is_err()); + assert_eq!(reactor.remove_callback(output, callback), Err(RemoveCallbackError::NonexistentCallback)); } assert!(reactor.set_value(input, 2).is_ok()); @@ -227,7 +227,7 @@ fn callbacks_should_only_be_called_once_even_if_multiple_dependencies_change() { let minus_one1 = reactor.create_compute(&[input], |v| v[0] - 1).unwrap(); let minus_one2 = reactor.create_compute(&[minus_one1], |v| v[0] - 1).unwrap(); let output = reactor.create_compute(&[plus_one, minus_one2], |v| v[0] * v[1]).unwrap(); - assert!(reactor.add_callback(output, |v| cb.callback_called(v)).is_ok()); + assert!(reactor.add_callback(output, |v| cb.callback_called(v)).is_some()); assert!(reactor.set_value(input, 4).is_ok()); cb.expect_to_have_been_called_with(10); } @@ -241,7 +241,7 @@ fn callbacks_should_not_be_called_if_dependencies_change_but_output_value_doesnt let plus_one = reactor.create_compute(&[input], |v| v[0] + 1).unwrap(); let minus_one = reactor.create_compute(&[input], |v| v[0] - 1).unwrap(); let always_two = reactor.create_compute(&[plus_one, minus_one], |v| v[0] - v[1]).unwrap(); - assert!(reactor.add_callback(always_two, |v| cb.callback_called(v)).is_ok()); + assert!(reactor.add_callback(always_two, |v| cb.callback_called(v)).is_some()); for i in 2..5 { assert!(reactor.set_value(input, i).is_ok()); cb.expect_not_to_have_been_called();