Skip to content

Commit

Permalink
Use Vec instead of Option<Vec>
Browse files Browse the repository at this point in the history
Discussion #325 (comment). Some([]) (empty vec) is a possible state from the type signature `Option<Vec<String>>` the state of Some/None is ambiguous without more information from the function. This code is easier to accidentally use the wrong logic in the consumer, but the correct implementation checks the direct properties we desire instead of relying on internal implementation of a function.

We could alternatively introduce a non-empty vector, there are several crates that provide this, but it's more overhead that needed here.
  • Loading branch information
schneems committed Sep 24, 2024
1 parent 7110b13 commit 6e2190d
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions buildpacks/ruby/src/layers/ruby_install_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,20 @@ pub(crate) fn handle(
),
},
restored_layer_action: &|old: &Metadata, _| {
if let Some(diff) = metadata_diff(old, &metadata) {
let diff = metadata_diff(old, &metadata);
if diff.is_empty() {
(
RestoredLayerAction::DeleteLayer,
format!("due to change(s): {}", SentenceList::new(&diff)),
RestoredLayerAction::KeepLayer,
"using cached version".to_string(),
)
} else {
(
RestoredLayerAction::KeepLayer,
"using cached version".to_string(),
RestoredLayerAction::DeleteLayer,
format!(
"due to {changes}: {differences}",
changes = if diff.len() > 1 { "changes" } else { "change" },
differences = SentenceList::new(&diff)
),
)
}
},
Expand Down Expand Up @@ -165,7 +170,7 @@ impl TryFrom<MetadataV1> for MetadataV2 {
}
}

fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option<Vec<String>> {
fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Vec<String> {
let mut differences = Vec::new();
let Metadata {
distro_name,
Expand Down Expand Up @@ -202,11 +207,7 @@ fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option<Vec<String>> {
));
}

if differences.is_empty() {
None
} else {
Some(differences)
}
differences
}

fn download_url(
Expand Down

0 comments on commit 6e2190d

Please sign in to comment.