From b2691d175c5ff360a8adcf8ed8bbd96e2dfb6bd3 Mon Sep 17 00:00:00 2001
From: "Kevin R. Thornton" <krthornt@uci.edu>
Date: Tue, 19 Jul 2022 02:06:25 -0700
Subject: [PATCH] refactor: Deduplicate code for adding node table rows.

---
 src/_macros.rs          | 58 +++++++++++++++++++++++++++++++++++++++
 src/node_table.rs       | 49 ++-------------------------------
 src/table_collection.rs | 61 ++++-------------------------------------
 3 files changed, 66 insertions(+), 102 deletions(-)

diff --git a/src/_macros.rs b/src/_macros.rs
index e2e4b5015..54e51a78b 100644
--- a/src/_macros.rs
+++ b/src/_macros.rs
@@ -576,6 +576,64 @@ macro_rules! build_owned_tables {
     };
 }
 
+macro_rules! node_table_add_row {
+    ($(#[$attr:meta])* => $name: ident, $self: ident, $table: ident $(, $table2: ident )?) => {
+        $(#[$attr])*
+        pub fn $name(
+            &mut $self,
+            flags: impl Into<$crate::NodeFlags>,
+            time: impl Into<$crate::Time>,
+            population: impl Into<$crate::PopulationId>,
+            individual: impl Into<$crate::IndividualId>,
+        ) -> Result<$crate::NodeId, $crate::TskitError> {
+            let rv = unsafe {
+                $crate::bindings::tsk_node_table_add_row(
+                    &mut (*$self.$table)$(.$table2)?,
+                    flags.into().bits(),
+                    time.into().0,
+                    population.into().0,
+                    individual.into().0,
+                    std::ptr::null(),
+                    0,
+                )
+            };
+
+            handle_tsk_return_value!(rv, rv.into())
+        }
+    };
+}
+
+macro_rules! node_table_add_row_with_metadata {
+    ($(#[$attr:meta])* => $name: ident, $self: ident, $table: ident $(, $table2: ident )?) => {
+        $(#[$attr])*
+        pub fn $name<M>(
+            &mut $self,
+            flags: impl Into<$crate::NodeFlags>,
+            time: impl Into<$crate::Time>,
+            population: impl Into<$crate::PopulationId>,
+            individual: impl Into<$crate::IndividualId>,
+            metadata: &M,
+        ) -> Result<$crate::NodeId, $crate::TskitError>
+        where
+            M: $crate::metadata::NodeMetadata,
+        {
+            let md = $crate::metadata::EncodedMetadata::new(metadata)?;
+            let rv = unsafe {
+                $crate::bindings::tsk_node_table_add_row(
+                    &mut (*$self.$table)$(.$table2)?,
+                    flags.into().bits(),
+                    time.into().0,
+                    population.into().0,
+                    individual.into().0,
+                    md.as_ptr(),
+                    md.len().into(),
+                )
+            };
+            handle_tsk_return_value!(rv, rv.into())
+        }
+    };
+}
+
 #[cfg(test)]
 mod test {
     use crate::error::TskitError;
diff --git a/src/node_table.rs b/src/node_table.rs
index a187a9a79..ad228de2a 100644
--- a/src/node_table.rs
+++ b/src/node_table.rs
@@ -293,53 +293,8 @@ pub struct OwnedNodeTable {
 }
 
 impl OwnedNodeTable {
-    pub fn add_row(
-        &mut self,
-        flags: impl Into<NodeFlags>,
-        time: impl Into<Time>,
-        population: impl Into<PopulationId>,
-        individual: impl Into<IndividualId>,
-    ) -> Result<NodeId, TskitError> {
-        let rv = unsafe {
-            ll_bindings::tsk_node_table_add_row(
-                &mut (*self.table),
-                flags.into().bits(),
-                time.into().0,
-                population.into().0,
-                individual.into().0,
-                std::ptr::null(),
-                0,
-            )
-        };
-
-        handle_tsk_return_value!(rv, rv.into())
-    }
-
-    pub fn add_row_with_metadata<M>(
-        &mut self,
-        flags: impl Into<NodeFlags>,
-        time: impl Into<Time>,
-        population: impl Into<PopulationId>,
-        individual: impl Into<IndividualId>,
-        metadata: &M,
-    ) -> Result<NodeId, TskitError>
-    where
-        M: crate::metadata::NodeMetadata,
-    {
-        let md = crate::metadata::EncodedMetadata::new(metadata)?;
-        let rv = unsafe {
-            ll_bindings::tsk_node_table_add_row(
-                &mut (*self.table),
-                flags.into().bits(),
-                time.into().0,
-                population.into().0,
-                individual.into().0,
-                md.as_ptr(),
-                md.len().into(),
-            )
-        };
-        handle_tsk_return_value!(rv, rv.into())
-    }
+    node_table_add_row!(=> add_row, self, table);
+    node_table_add_row_with_metadata!(=> add_row_with_metadata, self, table);
 }
 
 build_owned_tables!(
diff --git a/src/table_collection.rs b/src/table_collection.rs
index 3be6940e2..4d308a6a6 100644
--- a/src/table_collection.rs
+++ b/src/table_collection.rs
@@ -11,7 +11,6 @@ use crate::IndividualTable;
 use crate::IndividualTableSortOptions;
 use crate::MigrationTable;
 use crate::MutationTable;
-use crate::NodeFlags;
 use crate::NodeTable;
 use crate::PopulationTable;
 use crate::Position;
@@ -515,33 +514,12 @@ impl TableCollection {
         handle_tsk_return_value!(rv, MigrationId(rv))
     }
 
+    node_table_add_row!(
     /// Add a row to the node table
-    pub fn add_node<
-        F: Into<NodeFlags>,
-        T: Into<Time>,
-        POP: Into<PopulationId>,
-        I: Into<IndividualId>,
-    >(
-        &mut self,
-        flags: F,
-        time: T,
-        population: POP,
-        individual: I,
-    ) -> Result<NodeId, TskitError> {
-        let rv = unsafe {
-            ll_bindings::tsk_node_table_add_row(
-                &mut (*self.as_mut_ptr()).nodes,
-                flags.into().bits(),
-                time.into().0,
-                population.into().0,
-                individual.into().0,
-                std::ptr::null(),
-                0,
-            )
-        };
+    => add_node, self, inner, nodes
+    );
 
-        handle_tsk_return_value!(rv, rv.into())
-    }
+    node_table_add_row_with_metadata!(
 
     /// Add a row with optional metadata to the node table
     ///
@@ -563,35 +541,7 @@ impl TableCollection {
     /// assert!(tables.add_node_with_metadata(0, 0.0, -1, -1, &metadata).is_ok());
     /// # }
     /// ```
-    pub fn add_node_with_metadata<
-        F: Into<NodeFlags>,
-        T: Into<Time>,
-        POP: Into<PopulationId>,
-        I: Into<IndividualId>,
-        M: NodeMetadata,
-    >(
-        &mut self,
-        flags: F,
-        time: T,
-        population: POP,
-        individual: I,
-        metadata: &M,
-    ) -> Result<NodeId, TskitError> {
-        let md = EncodedMetadata::new(metadata)?;
-        let rv = unsafe {
-            ll_bindings::tsk_node_table_add_row(
-                &mut (*self.as_mut_ptr()).nodes,
-                flags.into().bits(),
-                time.into().0,
-                population.into().0,
-                individual.into().0,
-                md.as_ptr(),
-                md.len().into(),
-            )
-        };
-
-        handle_tsk_return_value!(rv, rv.into())
-    }
+    => add_node_with_metadata, self, inner, nodes);
 
     /// Add a row to the site table
     pub fn add_site<P: Into<Position>>(
@@ -1226,6 +1176,7 @@ impl crate::traits::NodeListGenerator for TableCollection {}
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::NodeFlags;
 
     fn make_small_table_collection() -> TableCollection {
         let mut tables = TableCollection::new(1000.).unwrap();