Change all db:: and tx::get_ methods to use fetch_

Ran commands:
`rg -l get_ src/ | xargs sed -i 's/get_/fetch_/g'
rg -l fetch_args src/ | xargs sed -i 's/fetch_args/get_args/g'
`
and fixed one comment referring to sqlite::Row::get and ::get_checked.

notes.md

@@ -526,3 +526,7 @@ other operations get rid of the &self parameter and just take a &DBTransaction a
 ---
 
 also, in server communication we need to either serialize the transaction operations rather than the dbbackend operations or include the current task when communicating with the server, because choosing the current task is nondeterministic. another option is to add the chosen task UUID to the operation log inside `tx::set_current_task` or something.
+
+---
+
+I happened to read https://robatwilliams.github.io/decent-code/ and there was a point made about using the word "get" for getters, and I agreed with it. So why am I using 'get' for a (relatively) expensive database query operation? I will change it.

src/commands/current.rs

@@ -21,7 +21,7 @@ fn format_category(task: &Task) -> String {
 
 impl Subcommand for Current {
     fn run(&self, db: &mut impl DBBackend) -> Result<Vec<String>, Error> {
-        let res = db.get_current_task()
+        let res = db.fetch_current_task()
             .map_err(|e| format_err!("Could not get current task from database. {}", e))?;
 
         if let Some(current) = res {

src/commands/list.rs

@@ -11,7 +11,7 @@ pub struct List;
 impl Subcommand for List {
     fn run(&self, db: &mut impl DBBackend) -> Result<Vec<String>, Error> {
 
-        let tasks = db.get_all_tasks()
+        let tasks = db.fetch_all_tasks()
             .map_err(|e| format_err!("Could not get tasks from database. {}", e))?;
 
         let mut output = vec!["Priority \t Task".to_string(),];

src/commands/mod.rs

@@ -97,7 +97,7 @@ impl TKZCmd {
             _ => unimplemented!(),
         };
 
-        let current_task = db.get_current_task()
+        let current_task = db.fetch_current_task()
             .map_err(|err| format_err!("Error getting current task while choosing new task after executing command: {}", err))?;
         // if there is no current task, choose a new one.
         if current_task.is_none() {
@@ -109,7 +109,7 @@ impl TKZCmd {
     }
 
     fn choose_new_current(&self, db: &mut impl DBBackend, task_p: f32, category_p: f32, break_cutoff: f32) -> Result<(), Error> {
-        let tasks = db.get_all_tasks()
+        let tasks = db.fetch_all_tasks()
             .map_err(|err| format_err!("Failed to get tasks while choosing new task after executing command: {}", err))?;
         let num_breaks = tasks.iter().filter(|&t| t.is_break()).count();
         let num_tasks = tasks.len() - num_breaks;

src/commands/test_dispatch.rs

@@ -23,7 +23,7 @@ fn test_runcmd_add_current() {
 
     cmd.run(&mut db, FIRST_TASK).expect("Add command failed");
 
-    let current = db.get_current_task().expect("Failed getting current task from db");
+    let current = db.fetch_current_task().expect("Failed getting current task from db");
     assert!(current.is_some(), "No current task after running Add command");
 
     let current = current.unwrap();
@@ -41,7 +41,7 @@ fn test_runcmd_add_multiple_current() {
 
     cmd.run(&mut db, FIRST_TASK).expect("Add command failed");
 
-    let current = db.get_current_task().expect("Failed getting current task from db");
+    let current = db.fetch_current_task().expect("Failed getting current task from db");
     assert!(current.is_some(), "No current task after running Add command");
 
     let current = current.unwrap();
@@ -52,7 +52,7 @@ fn test_runcmd_add_multiple_current() {
         let cmd = add_from_task(&other_task);
         cmd.run(&mut db, FIRST_TASK).expect("Add command failed");
 
-        let current = db.get_current_task().expect("Failed getting current task from db");
+        let current = db.fetch_current_task().expect("Failed getting current task from db");
         assert!(current.is_some(), "No current task after running Add command");
 
         let current = current.unwrap();
@@ -72,7 +72,7 @@ proptest! {
 
         cmd.run(&mut db, FIRST_TASK).expect("Add command failed");
 
-        let current = db.get_current_task().expect("Failed getting current task from db");
+        let current = db.fetch_current_task().expect("Failed getting current task from db");
         prop_assert!(current.is_some(), "No current task after running Add command");
 
         let current = current.unwrap();
@@ -90,7 +90,7 @@ proptest! {
 
         cmd.run(&mut db, FIRST_TASK).expect("Add command failed");
 
-        let current = db.get_current_task().expect("Failed getting current task from db");
+        let current = db.fetch_current_task().expect("Failed getting current task from db");
         prop_assert!(current.is_some(), "No current task after running Add command");
 
         let current = current.unwrap();
@@ -101,7 +101,7 @@ proptest! {
             let cmd = add_from_task(&other_task);
             cmd.run(&mut db, FIRST_TASK).expect("Add command failed");
 
-            let current = db.get_current_task().expect("Failed getting current task from db");
+            let current = db.fetch_current_task().expect("Failed getting current task from db");
             prop_assert!(current.is_some(), "No current task after running Add command");
 
             let current = current.unwrap();

src/db/backend.rs

@@ -15,7 +15,7 @@ pub trait DBBackend {
     fn add_task(&mut self, task: &Task) -> Result<(), Error>;
 
     /// Return a `Vec` of all tasks from the database
-    fn get_all_tasks(&mut self) -> Result<Vec<Task>, Error>;
+    fn fetch_all_tasks(&mut self) -> Result<Vec<Task>, Error>;
 
     /// Choose a new task at random to be the current task. Note that if the existing current task is not
     /// removed, it may be selected again. `p` is a parameter that must be between 0 and 1 which
@@ -29,7 +29,7 @@ pub trait DBBackend {
 
     /// Returns the currently selected task if there is one, or None if there are no tasks in the
     /// database.  This function should never return None if there are tasks in the database.
-    fn get_current_task(&mut self) -> Result<Option<Task>, Error>;
+    fn fetch_current_task(&mut self) -> Result<Option<Task>, Error>;
 
     /// Close the database. This is not really required due to the implementation of Drop for the
     /// Sqlite connection, but it might be necessary for other implementations e.g. a mock.
@@ -41,7 +41,7 @@ pub trait DBBackend {
 // the network) to make a db error type so that we can distinguish them - eg retrying a network
 // query later.
 
-// TODO use get_checked instead of get when deserializing rows. i feel like there should be a
+// TODO use get_checked() instead of get() when deserializing rows. i feel like there should be a
 // functional way to do it so that we get a Result<T> at the end. in particular the error messages
 // are bad - they should be something like format_err!("error trying to deserialize foo field from database
 // row: {}", e) but there should be a way to do it without writing that out every time, instead
@@ -82,11 +82,11 @@ impl DBBackend for SqliteBackend {
         Ok(())
     }
 
-    fn get_all_tasks(&mut self) -> Result<Vec<Task>, Error> {
+    fn fetch_all_tasks(&mut self) -> Result<Vec<Task>, Error> {
         let tx = self.transaction()?;
-        let tasks = tx.get_tasks()
+        let tasks = tx.fetch_tasks()
             .map_err(|e| format_err!("Failed to get tasks during transaction: {}", e))?;
-        let breaks = tx.get_breaks()
+        let breaks = tx.fetch_breaks()
             .map_err(|e| format_err!("Failed to get break tasks during transaction: {}", e))?;
 
         // chain tasks followed by breaks into single vector
@@ -109,11 +109,11 @@ impl DBBackend for SqliteBackend {
         let tx = self.transaction()?;
         let tasks = {
             if reward {
-                tx.get_breaks()
+                tx.fetch_breaks()
                     .map_err(|e| format_err!("Failed to get break tasks during transaction: {}", e))?
             }
             else {
-                tx.get_tasks()
+                tx.fetch_tasks()
                     .map_err(|e| format_err!("Failed to get tasks during transaction: {}", e))?
             }
         };
@@ -132,7 +132,7 @@ impl DBBackend for SqliteBackend {
         Ok(())
     }
 
-    fn get_current_task(&mut self) -> Result<Option<Task>, Error> {
+    fn fetch_current_task(&mut self) -> Result<Option<Task>, Error> {
         let tx = self.connection.transaction()?;
 
         // we will be able to return the task directly without declaring outside the scope once we

src/db/tests/get_current.rs → src/db/tests/fetch_current.rs

@@ -11,7 +11,7 @@ use proptest::test_runner::TestCaseError;
 fn assert_task_at_p(db: &mut SqliteBackend, p: f32, expected_task: &Task, msg: &str) {
     db.choose_current_task(p, expected_task.is_break()).expect("Failed choosing current task");
 
-    let res = db.get_current_task();
+    let res = db.fetch_current_task();
     assert!(res.is_ok(), "Getting current task failed: {}", res.unwrap_err());
 
     let task_opt = res.unwrap();
@@ -25,7 +25,7 @@ fn assert_task_at_p(db: &mut SqliteBackend, p: f32, expected_task: &Task, msg: &
 fn prop_assert_task_at_p(db: &mut SqliteBackend, p: f32, expected_task: &Task, msg: &str) -> Result<(), TestCaseError> {
     db.choose_current_task(p, expected_task.is_break()).expect("Failed choosing current task");
 
-    let res = db.get_current_task();
+    let res = db.fetch_current_task();
     prop_assert!(res.is_ok(), "Getting current task failed: {}", res.unwrap_err());
 
     let task_opt = res.unwrap();
@@ -39,18 +39,18 @@ fn prop_assert_task_at_p(db: &mut SqliteBackend, p: f32, expected_task: &Task, m
 
 
 #[test]
-fn test_db_get_current_no_tasks() {
+fn test_db_fetch_current_no_tasks() {
     let mut db = open_test_db();
 
-    let res = db.get_current_task();
+    let res = db.fetch_current_task();
     assert!(res.is_ok(), "Getting current task failed: {}", res.unwrap_err());
 
     let no_task = res.unwrap();
     assert!(no_task.is_none(), "Without adding a task, somehow there was a current task: {:?}", no_task);
 }
 
 #[test]
-fn test_db_get_current_one_task() {
+fn test_db_fetch_current_one_task() {
     let mut db = open_test_db();
 
     db.add_task(&example_task_1()).expect("Failed adding task to db in test");
@@ -67,7 +67,7 @@ fn test_db_get_current_one_task() {
 }
 
 #[test]
-fn test_db_get_current_one_break() {
+fn test_db_fetch_current_one_break() {
     let mut db = open_test_db();
 
     db.add_task(&example_task_break_1()).expect("Failed adding task to db in test");
@@ -84,7 +84,7 @@ fn test_db_get_current_one_break() {
 }
 
 #[test]
-fn test_db_get_current_task_vs_break() {
+fn test_db_fetch_current_task_vs_break() {
     let mut db = open_test_db();
 
     db.add_task(&example_task_1()).expect("Failed adding task to db in test");
@@ -104,7 +104,7 @@ fn test_db_get_current_task_vs_break() {
 }
 
 #[test]
-fn test_db_get_current_ordering_two() {
+fn test_db_fetch_current_ordering_two() {
     let mut db = open_test_db();
 
     // two tasks with equal priority but names are different, second is exactly the same but ends
@@ -124,7 +124,7 @@ fn test_db_get_current_ordering_two() {
 }
 
 #[test]
-fn test_db_get_current_two_max_u32() {
+fn test_db_fetch_current_two_max_u32() {
     let mut db = open_test_db();
 
     let task1 = Task::from_parts("a".to_string(), u32::max_value(), false).unwrap();
@@ -146,7 +146,7 @@ fn test_db_get_current_two_max_u32() {
 // TODO this test could be better probably. 
 proptest! {
     #[test]
-    fn test_db_get_current_arb(tasks in arb_task_list()) {
+    fn test_db_fetch_current_arb(tasks in arb_task_list()) {
         let mut db = open_test_db();
 
         for task in &tasks {

src/db/tests/list.rs

@@ -10,7 +10,7 @@ fn test_db_list_empty() {
     let mut db = open_test_db();
 
     // get nothing from db
-    let res = db.get_all_tasks();
+    let res = db.fetch_all_tasks();
     assert!(res.is_ok(), "Tasks could not be retrieved: {:?}", res.unwrap_err());
     let db_tasks = res.unwrap();
 
@@ -25,7 +25,7 @@ fn test_db_list_invalid_task_empty() {
     let task = Task::example_invalid_empty_desc();
     db.add_task(&task).expect("Adding task failed");
 
-    let res = db.get_all_tasks();
+    let res = db.fetch_all_tasks();
     assert!(res.is_err(), "No error when trying to deserialize invalid task: {:?}", res);
 
     let err = res.unwrap_err();
@@ -39,7 +39,7 @@ fn test_db_list_invalid_task_zero_priority() {
     let task = Task::example_invalid_zero_priority();
     db.add_task(&task).expect("Adding task failed");
 
-    let res = db.get_all_tasks();
+    let res = db.fetch_all_tasks();
     assert!(res.is_err(), "No error when trying to deserialize invalid task: {:?}", res);
 
     let err = res.unwrap_err();
@@ -60,7 +60,7 @@ fn test_db_list_added_manually() {
     }
 
     // get tasks back from db
-    let res = db.get_all_tasks();
+    let res = db.fetch_all_tasks();
     assert!(res.is_ok(), "Tasks could not be retrieved: {:?}", res.unwrap_err());
     let db_tasks = res.unwrap();
 
@@ -86,7 +86,7 @@ proptest! {
         }
 
         // get tasks back
-        let res = db.get_all_tasks();
+        let res = db.fetch_all_tasks();
         prop_assert!(res.is_ok(), "Tasks could not be retrieved: {:?}", res.unwrap_err());
         let db_tasks = res.unwrap();
 

src/db/tests/mod.rs

@@ -8,7 +8,7 @@ use crate::db::{SqliteBackend, DBBackend};
 
 mod add;
 mod choose_current;
-mod get_current;
+mod fetch_current;
 mod list;
 
 // utility functions

src/db/transaction/mod.rs

@@ -24,17 +24,17 @@ pub trait DBTransaction {
     fn add_task(&self, task: &Task) -> Result<(), Error>;
 
     /// Return a `Vec` of all tasks (non-breaks) from the database. 
-    fn get_tasks(&self) -> Result<Vec<(RowId, Task)>, Error>;
+    fn fetch_tasks(&self) -> Result<Vec<(RowId, Task)>, Error>;
 
     /// Return a `Vec` of all breaks from the database.
-    fn get_breaks(&self) -> Result<Vec<(RowId, Task)>, Error>;
+    fn fetch_breaks(&self) -> Result<Vec<(RowId, Task)>, Error>;
 
     /// Set the current task to be the task with id `id`.
     fn set_current_task(&self, id: &RowId) -> Result<(), Error>;
 
     /// Returns the currently selected task if there is one, or None if there are no tasks in the
     /// database. This function should never return None if there are tasks in the database.
-    fn get_current_task(&self) -> Result<Option<Task>, Error>;
+    fn fetch_current_task(&self) -> Result<Option<Task>, Error>;
 
     /// Returns the `RowId` of the currently selected task if there is one, or None if there are no
     /// tasks in the database. The current task is then set to None, but the task itself is not
@@ -73,7 +73,7 @@ impl<'conn> DBTransaction for SqliteTransaction<'conn> {
         Ok(())
     }
 
-    fn get_tasks(&self) -> Result<Vec<(RowId, Task)>, Error> {
+    fn fetch_tasks(&self) -> Result<Vec<(RowId, Task)>, Error> {
         let tx = &self.transaction;
 
         let mut tasks = Vec::new();
@@ -101,7 +101,7 @@ impl<'conn> DBTransaction for SqliteTransaction<'conn> {
         Ok(tasks)
     }
 
-    fn get_breaks(&self) -> Result<Vec<(RowId, Task)>, Error> {
+    fn fetch_breaks(&self) -> Result<Vec<(RowId, Task)>, Error> {
         let tx = &self.transaction;
 
         let mut tasks = Vec::new();
@@ -148,7 +148,7 @@ impl<'conn> DBTransaction for SqliteTransaction<'conn> {
 
     }
 
-    fn get_current_task(&self) -> Result<Option<Task>, Error> {
+    fn fetch_current_task(&self) -> Result<Option<Task>, Error> {
         let tx = &self.transaction;
         let mut stmt = tx.prepare_cached(
             "SELECT task, priority, category