From f067c8b79a9f710fbc3075e76507fe9fb178c667 Mon Sep 17 00:00:00 2001
From: Ian Goldberg <iang@uwaterloo.ca>
Date: Tue, 27 Apr 2021 12:59:29 -0400
Subject: [PATCH] Use a custom enum instead of Result<(),()> in DupFilter

---
 crates/lox-library/src/dup_filter.rs | 30 +++++++++++++++++++--------
 crates/lox-library/tests/tests.rs    | 31 ++++++++++++++--------------
 2 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/crates/lox-library/src/dup_filter.rs b/crates/lox-library/src/dup_filter.rs
index 72f00df..b128261 100644
--- a/crates/lox-library/src/dup_filter.rs
+++ b/crates/lox-library/src/dup_filter.rs
@@ -14,21 +14,33 @@ pub struct DupFilter<IdType> {
     seen_table: HashMap<IdType, ()>,
 }
 
+/// A return type indicating whether the item was fresh (not previously
+/// seen) or previously seen
+#[derive(PartialEq, Eq, Debug)]
+pub enum SeenType {
+    Fresh,
+    Seen,
+}
+
 impl<IdType: Hash + Eq + Copy> DupFilter<IdType> {
     /// Check to see if the id is in the seen table, but do not add it
-    /// to the seen table.  Return true if it is already in the table,
-    /// false if not.
-    pub fn check(&self, id: &IdType) -> bool {
-        self.seen_table.contains_key(id)
+    /// to the seen table.  Return Seen if it is already in the table,
+    /// Fresh if not.
+    pub fn check(&self, id: &IdType) -> SeenType {
+        if self.seen_table.contains_key(id) {
+            SeenType::Seen
+        } else {
+            SeenType::Fresh
+        }
     }
 
     /// As atomically as possible, check to see if the id is in the seen
-    /// table, and add it if not.  Return Ok(()) if it was not already
-    /// in the table, and Err(()) if it was.
-    pub fn filter(&mut self, id: &IdType) -> Result<(), ()> {
+    /// table, and add it if not.  Return Fresh if it was not already
+    /// in the table, and Seen if it was.
+    pub fn filter(&mut self, id: &IdType) -> SeenType {
         match self.seen_table.insert(*id, ()) {
-            None => Ok(()),
-            Some(()) => Err(()),
+            None => SeenType::Fresh,
+            Some(()) => SeenType::Seen,
         }
     }
 }
diff --git a/crates/lox-library/tests/tests.rs b/crates/lox-library/tests/tests.rs
index ad6bc4b..ce497fa 100644
--- a/crates/lox-library/tests/tests.rs
+++ b/crates/lox-library/tests/tests.rs
@@ -1,4 +1,5 @@
 use lox::dup_filter;
+use lox::dup_filter::SeenType::{Fresh, Seen};
 use lox::BridgeDb;
 
 use curve25519_dalek::scalar::Scalar;
@@ -23,23 +24,23 @@ fn test_dup_filter() {
     let s4 = Scalar::random(&mut rng);
     let s5 = Scalar::random(&mut rng);
     // Check basic behaviour
-    assert_eq!(df1.check(&s1), false);
-    assert_eq!(df1.filter(&s1), Ok(()));
-    assert_eq!(df1.check(&s1), true);
-    assert_eq!(df1.filter(&s1), Err(()));
+    assert_eq!(df1.check(&s1), Fresh);
+    assert_eq!(df1.filter(&s1), Fresh);
+    assert_eq!(df1.check(&s1), Seen);
+    assert_eq!(df1.filter(&s1), Seen);
     // Ensure different instances of DupFilter have different tables
-    assert_eq!(df2.check(&s1), false);
-    assert_eq!(df2.filter(&s1), Ok(()));
-    assert_eq!(df2.filter(&s1), Err(()));
-    assert_eq!(df2.check(&s1), true);
+    assert_eq!(df2.check(&s1), Fresh);
+    assert_eq!(df2.filter(&s1), Fresh);
+    assert_eq!(df2.filter(&s1), Seen);
+    assert_eq!(df2.check(&s1), Seen);
     // Check multiple ids
-    assert_eq!(df1.check(&s2), false);
-    assert_eq!(df1.filter(&s3), Ok(()));
-    assert_eq!(df1.filter(&s4), Ok(()));
-    assert_eq!(df1.filter(&s3), Err(()));
-    assert_eq!(df1.check(&s1), true);
-    assert_eq!(df1.filter(&s1), Err(()));
-    assert_eq!(df1.filter(&s5), Ok(()));
+    assert_eq!(df1.check(&s2), Fresh);
+    assert_eq!(df1.filter(&s3), Fresh);
+    assert_eq!(df1.filter(&s4), Fresh);
+    assert_eq!(df1.filter(&s3), Seen);
+    assert_eq!(df1.check(&s1), Seen);
+    assert_eq!(df1.filter(&s1), Seen);
+    assert_eq!(df1.filter(&s5), Fresh);
     println!("df1 = {:?}", df1);
     println!("df2 = {:?}", df2);
 }