refactor: remove redundant get_ordering fn, add tests

src/app/data_farmer.rs

@@ -1,20 +1,22 @@
+//! In charge of cleaning, processing, and managing data.  I couldn't think of
+//! a better name for the file.  Since I called data collection "harvesting",
+//! then this is the farmer I guess.
+//!
+//! Essentially the main goal is to shift the initial calculation and distribution
+//! of joiner points and data to one central location that will only do it
+//! *once* upon receiving the data --- as opposed to doing it on canvas draw,
+//! which will be a costly process.
+//!
+//! This will also handle the *cleaning* of stale data.  That should be done
+//! in some manner (timer on another thread, some loop) that will occasionally
+//! call the purging function.  Failure to do so *will* result in a growing
+//! memory usage and higher CPU usage - you will be trying to process more and
+//! more points as this is used!
+
+use once_cell::sync::Lazy;
+
 use fxhash::FxHashMap;
 use itertools::Itertools;
-/// In charge of cleaning, processing, and managing data.  I couldn't think of
-/// a better name for the file.  Since I called data collection "harvesting",
-/// then this is the farmer I guess.
-///
-/// Essentially the main goal is to shift the initial calculation and distribution
-/// of joiner points and data to one central location that will only do it
-/// *once* upon receiving the data --- as opposed to doing it on canvas draw,
-/// which will be a costly process.
-///
-/// This will also handle the *cleaning* of stale data.  That should be done
-/// in some manner (timer on another thread, some loop) that will occasionally
-/// call the purging function.  Failure to do so *will* result in a growing
-/// memory usage and higher CPU usage - you will be trying to process more and
-/// more points as this is used!
-use once_cell::sync::Lazy;
 
 use std::{time::Instant, vec::Vec};
 

src/app/states/table_state.rs

@@ -189,7 +189,7 @@ impl<H: TableComponentHeader> TableComponentColumn<H> {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum SortOrder {
     Ascending,
     Descending,
@@ -259,7 +259,7 @@ impl SortableState {
             .iter()
             .map(|width| {
                 let range_start = start;
-                let range_end = start + width + 1;
+                let range_end = start + width + 1; // +1 for the gap b/w cols.
                 start = range_end;
                 range_start..range_end
             })
@@ -626,7 +626,57 @@ mod test {
     }
 
     #[test]
-    fn test_row_width_boundary_creation() {
+    fn test_visual_index_selection() {
-        // FIXME: [TEST] finish this
+        let mut state = SortableState::new(
+            0,
+            SortOrder::Ascending,
+            vec![SortOrder::Ascending, SortOrder::Descending],
+        );
+
+        const X_OFFSET: u16 = 10;
+        const Y_OFFSET: u16 = 15;
+        state.update_visual_index(Rect::new(X_OFFSET, Y_OFFSET, 20, 15), &[4, 14]);
+
+        #[track_caller]
+        fn test_selection(
+            state: &mut SortableState, from_x_offset: u16, from_y_offset: u16,
+            result: (Option<usize>, SortOrder),
+        ) {
+            assert_eq!(
+                state.try_select_location(X_OFFSET + from_x_offset, Y_OFFSET + from_y_offset),
+                result.0
+            );
+            assert_eq!(state.order, result.1);
+        }
+
+        use SortOrder::*;
+
+        // Clicking on these don't do anything, so don't show any change.
+        test_selection(&mut state, 5, 1, (None, Ascending));
+        test_selection(&mut state, 21, 0, (None, Ascending));
+
+        // Clicking on the first column should toggle it as it is already selected.
+        test_selection(&mut state, 3, 0, (Some(0), Descending));
+
+        // Clicking on the first column should toggle it again as it is already selected.
+        test_selection(&mut state, 4, 0, (Some(0), Ascending));
+
+        // Clicking on second column should select and switch to the descending ordering as that is its default.
+        test_selection(&mut state, 5, 0, (Some(1), Descending));
+
+        // Clicking on second column should toggle it.
+        test_selection(&mut state, 19, 0, (Some(1), Ascending));
+
+        // Overshoot, should not do anything.
+        test_selection(&mut state, 20, 0, (None, Ascending));
+
+        // Further overshoot, should not do anything.
+        test_selection(&mut state, 25, 0, (None, Ascending));
+
+        // Go back to first column, should be ascending to match default for index 0.
+        test_selection(&mut state, 3, 0, (Some(0), Ascending));
+
+        // Click on first column should then go to descending as it is already selected and ascending.
+        test_selection(&mut state, 3, 0, (Some(0), Descending));
     }
 }

src/canvas/widgets/network_graph.rs

@@ -256,7 +256,7 @@ fn get_max_entry(
         (None, Some(filtered_tx)) => {
             match filtered_tx
                 .iter()
-                .max_by(|(_, data_a), (_, data_b)| get_ordering(data_a, data_b, false))
+                .max_by(|(_, data_a), (_, data_b)| partial_ordering(data_a, data_b))
             {
                 Some((best_time, max_val)) => {
                     if *max_val == 0.0 {
@@ -277,7 +277,7 @@ fn get_max_entry(
         (Some(filtered_rx), None) => {
             match filtered_rx
                 .iter()
-                .max_by(|(_, data_a), (_, data_b)| get_ordering(data_a, data_b, false))
+                .max_by(|(_, data_a), (_, data_b)| partial_ordering(data_a, data_b))
             {
                 Some((best_time, max_val)) => {
                     if *max_val == 0.0 {
@@ -299,7 +299,7 @@ fn get_max_entry(
             match filtered_rx
                 .iter()
                 .chain(filtered_tx)
-                .max_by(|(_, data_a), (_, data_b)| get_ordering(data_a, data_b, false))
+                .max_by(|(_, data_a), (_, data_b)| partial_ordering(data_a, data_b))
             {
                 Some((best_time, max_val)) => {
                     if *max_val == 0.0 {

src/utils/gen_util.rs

@@ -112,49 +112,25 @@ pub fn partial_ordering_rev<T: std::cmp::PartialOrd>(a: T, b: T) -> Ordering {
     a.partial_cmp(&b).unwrap_or(Ordering::Equal).reverse()
 }
 
-/// Gotta get partial ordering?  No problem, here's something to deal with it~
-pub fn get_ordering<T: std::cmp::PartialOrd>(
-    a_val: T, b_val: T, reverse_order: bool,
-) -> std::cmp::Ordering {
-    // FIXME: Maybe we can just delete this entirely and change references to use partial_ordering...
-    match a_val.partial_cmp(&b_val) {
-        Some(x) => match x {
-            Ordering::Greater => {
-                if reverse_order {
-                    std::cmp::Ordering::Less
-                } else {
-                    std::cmp::Ordering::Greater
-                }
-            }
-            Ordering::Less => {
-                if reverse_order {
-                    std::cmp::Ordering::Greater
-                } else {
-                    std::cmp::Ordering::Less
-                }
-            }
-            Ordering::Equal => Ordering::Equal,
-        },
-        None => Ordering::Equal,
-    }
-}
-
 #[cfg(test)]
 mod test {
-    // use super::*;
+    use super::*;
 
     #[test]
     fn test_sort_partial_fn() {
-        // FIXME: Do this
+        let mut x = vec![9, 5, 20, 15, 10, 5];
-    }
+        let mut y = vec![1.0, 15.0, -1.0, -100.0, -100.1, 16.15, -100.0];
 
-    #[test]
+        x.sort_by(|a, b| sort_partial_fn(false)(a, b));
-    fn test_partial_ordering() {
+        assert_eq!(x, vec![5, 5, 9, 10, 15, 20]);
-        // FIXME: Do this
-    }
 
-    #[test]
+        x.sort_by(|a, b| sort_partial_fn(true)(a, b));
-    fn test_reverse_partial_ordering() {
+        assert_eq!(x, vec![20, 15, 10, 9, 5, 5]);
-        // FIXME: Do this
+
+        y.sort_by(|a, b| sort_partial_fn(false)(a, b));
+        assert_eq!(y, vec![-100.1, -100.0, -100.0, -1.0, 1.0, 15.0, 16.15]);
+
+        y.sort_by(|a, b| sort_partial_fn(true)(a, b));
+        assert_eq!(y, vec![16.15, 15.0, 1.0, -1.0, -100.0, -100.0, -100.1]);
     }
 }