fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

refactor: make active_view and axis_of infallible

Browse Source 
Both functions previously returned Option despite their invariants
guaranteeing a value: active_view always names an existing view
(maintained by new/switch_view/delete_view), and axis_of only returns
None for categories never registered with the view (a programming error).

Callers no longer need to handle the impossible None case, eliminating
~15 match/if-let Option guards across app.rs, dispatch.rs, grid.rs,
tile_bar.rs, and category_panel.rs.

Also adds Model::evaluate_f64 (returns 0.0 for empty cells) and collapses
the double match-on-axis pattern in tile_bar/category_panel into a single
axis_display(Axis) helper.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Ed L
2026-03-24 09:00:25 -07:00
parent a2e519efcc
commit 6038cb2d81
9 changed files with 168 additions and 208 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/view/layout.rs
View File

@ -114,7 +114,7 @@ mod tests {
#[test]
fn row_and_col_counts_match_item_counts() {
let m = two_cat_model();
let layout = GridLayout::new(&m, m.active_view().unwrap());
let layout = GridLayout::new(&m, m.active_view());
assert_eq!(layout.row_count(), 2); // Food, Clothing
assert_eq!(layout.col_count(), 2); // Jan, Feb
}
@ -122,7 +122,7 @@ mod tests {
#[test]
fn cell_key_encodes_correct_coordinates() {
let m = two_cat_model();
let layout = GridLayout::new(&m, m.active_view().unwrap());
let layout = GridLayout::new(&m, m.active_view());
// row 0 = Food, col 1 = Feb
let key = layout.cell_key(0, 1).unwrap();
assert_eq!(key, coord(&[("Month", "Feb"), ("Type", "Food")]));
@ -131,7 +131,7 @@ mod tests {
#[test]
fn cell_key_out_of_bounds_returns_none() {
let m = two_cat_model();
let layout = GridLayout::new(&m, m.active_view().unwrap());
let layout = GridLayout::new(&m, m.active_view());
assert!(layout.cell_key(99, 0).is_none());
assert!(layout.cell_key(0, 99).is_none());
}
@ -146,8 +146,8 @@ mod tests {
m.category_mut("Month").unwrap().add_item("Jan");
m.category_mut("Region").unwrap().add_item("East");
m.category_mut("Region").unwrap().add_item("West");
m.active_view_mut().unwrap().set_page_selection("Region", "West");
let layout = GridLayout::new(&m, m.active_view().unwrap());
m.active_view_mut().set_page_selection("Region", "West");
let layout = GridLayout::new(&m, m.active_view());
let key = layout.cell_key(0, 0).unwrap();
assert_eq!(key.get("Region"), Some("West"));
}
@ -159,7 +159,7 @@ mod tests {
coord(&[("Month", "Feb"), ("Type", "Clothing")]),
CellValue::Number(42.0),
);
let layout = GridLayout::new(&m, m.active_view().unwrap());
let layout = GridLayout::new(&m, m.active_view());
// Clothing = row 1, Feb = col 1
let key = layout.cell_key(1, 1).unwrap();
assert_eq!(m.evaluate(&key), Some(CellValue::Number(42.0)));
@ -174,8 +174,8 @@ mod tests {
m.category_mut("Type").unwrap().add_item("Food");
m.category_mut("Month").unwrap().add_item("Jan");
m.category_mut("Year").unwrap().add_item("2025");
m.active_view_mut().unwrap().set_axis("Year", crate::view::Axis::Column);
let layout = GridLayout::new(&m, m.active_view().unwrap());
m.active_view_mut().set_axis("Year", crate::view::Axis::Column);
let layout = GridLayout::new(&m, m.active_view());
assert_eq!(layout.col_label(0), "Jan/2025");
}
}

42
src/view/view.rs
View File

@ -61,8 +61,9 @@ impl View {
}
}
pub fn axis_of(&self, cat_name: &str) -> Option<Axis> {
self.category_axes.get(cat_name).copied()
pub fn axis_of(&self, cat_name: &str) -> Axis {
*self.category_axes.get(cat_name)
.expect("axis_of called for category not registered with this view")
}
pub fn categories_on(&self, axis: Axis) -> Vec<&str> {
@ -114,9 +115,9 @@ impl View {
/// Cycle axis for a category: Row → Column → Page → Row
pub fn cycle_axis(&mut self, cat_name: &str) {
let next = match self.axis_of(cat_name) {
Some(Axis::Row) | None => Axis::Column,
Some(Axis::Column) => Axis::Page,
Some(Axis::Page) => Axis::Row,
Axis::Row => Axis::Column,
Axis::Column => Axis::Page,
Axis::Page => Axis::Row,
};
self.set_axis(cat_name, next);
self.selected = (0, 0);
@ -139,27 +140,27 @@ mod tests {
#[test]
fn first_category_assigned_to_row() {
let v = view_with_cats(&["Region"]);
assert_eq!(v.axis_of("Region"), Some(Axis::Row));
assert_eq!(v.axis_of("Region"), Axis::Row);
}
#[test]
fn second_category_assigned_to_column() {
let v = view_with_cats(&["Region", "Product"]);
assert_eq!(v.axis_of("Product"), Some(Axis::Column));
assert_eq!(v.axis_of("Product"), Axis::Column);
}
#[test]
fn third_and_later_categories_assigned_to_page() {
let v = view_with_cats(&["Region", "Product", "Time", "Scenario"]);
assert_eq!(v.axis_of("Time"), Some(Axis::Page));
assert_eq!(v.axis_of("Scenario"), Some(Axis::Page));
assert_eq!(v.axis_of("Time"), Axis::Page);
assert_eq!(v.axis_of("Scenario"), Axis::Page);
}
#[test]
fn set_axis_changes_assignment() {
let mut v = view_with_cats(&["Region", "Product"]);
v.set_axis("Region", Axis::Column);
assert_eq!(v.axis_of("Region"), Some(Axis::Column));
assert_eq!(v.axis_of("Region"), Axis::Column);
}
#[test]
@ -171,9 +172,10 @@ mod tests {
}
#[test]
fn axis_of_unknown_category_returns_none() {
#[should_panic(expected = "axis_of called for category not registered")]
fn axis_of_unknown_category_panics() {
let v = View::new("Test");
assert_eq!(v.axis_of("Ghost"), None);
v.axis_of("Ghost");
}
#[test]
@ -208,7 +210,7 @@ mod tests {
fn cycle_axis_row_to_column() {
let mut v = view_with_cats(&["Region", "Product"]);
v.cycle_axis("Region");
assert_eq!(v.axis_of("Region"), Some(Axis::Column));
assert_eq!(v.axis_of("Region"), Axis::Column);
}
#[test]
@ -216,14 +218,14 @@ mod tests {
let mut v = view_with_cats(&["Region", "Product"]);
v.set_axis("Product", Axis::Column);
v.cycle_axis("Product");
assert_eq!(v.axis_of("Product"), Some(Axis::Page));
assert_eq!(v.axis_of("Product"), Axis::Page);
}
#[test]
fn cycle_axis_page_to_row() {
let mut v = view_with_cats(&["Region", "Product", "Time"]);
v.cycle_axis("Time");
assert_eq!(v.axis_of("Time"), Some(Axis::Row));
assert_eq!(v.axis_of("Time"), Axis::Row);
}
#[test]
@ -259,9 +261,7 @@ mod prop_tests {
for c in &cats { v.on_category_added(c); }
for c in &cats {
let axis = v.axis_of(c);
prop_assert!(axis.is_some(),
"category '{}' should be assigned after on_category_added", c);
let on_axis = v.categories_on(axis.unwrap());
let on_axis = v.categories_on(axis);
prop_assert!(on_axis.contains(&c.as_str()),
"categories_on({:?}) should contain '{}'", axis, c);
}
@ -287,9 +287,9 @@ mod prop_tests {
fn on_category_added_idempotent(cats in unique_cat_names()) {
let mut v = View::new("T");
for c in &cats { v.on_category_added(c); }
let axes_before: Vec<Option<Axis>> = cats.iter().map(|c| v.axis_of(c)).collect();
let axes_before: Vec<Axis> = cats.iter().map(|c| v.axis_of(c)).collect();
for c in &cats { v.on_category_added(c); }
let axes_after: Vec<Option<Axis>> = cats.iter().map(|c| v.axis_of(c)).collect();
let axes_after: Vec<Axis> = cats.iter().map(|c| v.axis_of(c)).collect();
prop_assert_eq!(axes_before, axes_after);
}
@ -305,7 +305,7 @@ mod prop_tests {
let idx = target_idx % cats.len();
let cat = &cats[idx];
v.set_axis(cat, axis);
prop_assert_eq!(v.axis_of(cat), Some(axis));
prop_assert_eq!(v.axis_of(cat), axis);
}
/// After set_axis(cat, X), cat is NOT in categories_on(Y) for Y ≠ X