Pass available_bridge by value rather than reference

Allows us to pop() from extra_bridges, which is less prone to errors than
calling both last() and then remove().
This commit is contained in:
Cecylia Bocovich 2024-03-09 19:14:14 -05:00
parent 2c396f63fa
commit e6b0cf842a
No known key found for this signature in database
GPG Key ID: 009DE379FD9B7B90
3 changed files with 16 additions and 21 deletions

View File

@ -251,12 +251,6 @@ impl LoxServerContext {
return_bridges return_bridges
} }
pub fn remove_single_bridge(&self) {
let mut extra_bridges = self.extra_bridges.lock().unwrap();
let length = extra_bridges.len();
_ = extra_bridges.remove(length - 1)
}
// Add extra_bridges to the Lox bridge table as open invitation bridges // Add extra_bridges to the Lox bridge table as open invitation bridges
// TODO: Add some consideration for whether or not bridges should be sorted as // TODO: Add some consideration for whether or not bridges should be sorted as
// open invitation buckets or hot spare buckets // open invitation buckets or hot spare buckets
@ -301,13 +295,14 @@ impl LoxServerContext {
// available bridges or from a spare bucket // available bridges or from a spare bucket
pub fn replace_with_new(&self, bridgeline: BridgeLine) -> lox_library::ReplaceSuccess { pub fn replace_with_new(&self, bridgeline: BridgeLine) -> lox_library::ReplaceSuccess {
let mut ba_obj = self.ba.lock().unwrap(); let mut ba_obj = self.ba.lock().unwrap();
let eb_obj = self.extra_bridges.lock().unwrap(); let mut eb_obj = self.extra_bridges.lock().unwrap();
let available_bridge = eb_obj.last(); let available_bridge = eb_obj.pop();
let result = ba_obj.bridge_replace(&bridgeline, available_bridge); let result = ba_obj.bridge_replace(&bridgeline, available_bridge);
// .last() doesn't actually remove the object so we still have to do that if the bridge was if result != lox_library::ReplaceSuccess::Replaced {
// replaced with an available bridge if let Some(bridge) = available_bridge {
if result == lox_library::ReplaceSuccess::Replaced && eb_obj.len() > 0 { // If available bridge wasn't removed, return it
self.remove_single_bridge(); eb_obj.push(bridge);
}
} }
result result
} }

View File

@ -540,7 +540,7 @@ impl BridgeAuth {
pub fn bridge_replace( pub fn bridge_replace(
&mut self, &mut self,
bridge: &BridgeLine, bridge: &BridgeLine,
available_bridge: Option<&BridgeLine>, available_bridge: Option<BridgeLine>,
) -> ReplaceSuccess { ) -> ReplaceSuccess {
let mut res = ReplaceSuccess::NotFound; let mut res = ReplaceSuccess::NotFound;
let reachable_bridges = &self.bridge_table.reachable.clone(); let reachable_bridges = &self.bridge_table.reachable.clone();
@ -554,12 +554,12 @@ impl BridgeAuth {
None => return ReplaceSuccess::NotFound, None => return ReplaceSuccess::NotFound,
}; };
assert!(bridgelines[*offset] == *bridge); assert!(bridgelines[*offset] == *bridge);
bridgelines[*offset] = *replacement; bridgelines[*offset] = replacement;
self.bridge_table.buckets.insert(*bucketnum, bridgelines); self.bridge_table.buckets.insert(*bucketnum, bridgelines);
// Remove the bridge from the reachable bridges and add new bridge // Remove the bridge from the reachable bridges and add new bridge
self.bridge_table self.bridge_table
.reachable .reachable
.insert(*replacement, positions.clone()); .insert(replacement, positions.clone());
// Remove the bridge from the bucket // Remove the bridge from the bucket
self.bridge_table.reachable.remove(bridge); self.bridge_table.reachable.remove(bridge);
} }

View File

@ -1063,21 +1063,21 @@ fn test_bridge_replace() {
match case { match case {
"not found" => { "not found" => {
// Case zero: bridge to be replaced is not in the bridgetable // Case zero: bridge to be replaced is not in the bridgetable
let random_bridgeline = &BridgeLine::random(); let random_bridgeline = BridgeLine::random();
assert!( assert!(
!th.ba.bridge_table.reachable.contains_key(random_bridgeline), !th.ba.bridge_table.reachable.contains_key(&random_bridgeline),
"Random bridgeline happens to be in the bridge_table (and should not be)" "Random bridgeline happens to be in the bridge_table (and should not be)"
); );
assert!( assert!(
th.ba th.ba
.bridge_replace(random_bridgeline, Some(random_bridgeline)) .bridge_replace(&random_bridgeline, Some(random_bridgeline))
== ReplaceSuccess::NotFound, == ReplaceSuccess::NotFound,
"Bridge should be marked as NotFound" "Bridge should be marked as NotFound"
); );
} }
"available" => { "available" => {
// Case one: available_bridge != null // Case one: available_bridge != null
let random_bridgeline = &BridgeLine::random(); let random_bridgeline = BridgeLine::random();
let unallocated_bridgeline = &BridgeLine::random(); let unallocated_bridgeline = &BridgeLine::random();
th.ba th.ba
.bridge_table .bridge_table
@ -1087,7 +1087,7 @@ fn test_bridge_replace() {
th.ba th.ba
.bridge_table .bridge_table
.reachable .reachable
.get(random_bridgeline) .get(&random_bridgeline)
.is_none(), .is_none(),
"Random bridge already in table" "Random bridge already in table"
); );
@ -1101,7 +1101,7 @@ fn test_bridge_replace() {
th.ba th.ba
.bridge_table .bridge_table
.reachable .reachable
.get(random_bridgeline) .get(&random_bridgeline)
.is_some(), .is_some(),
"Replacement bridge not added to reachable bridges" "Replacement bridge not added to reachable bridges"
); );