From b18b3cef3ef4f4545e8076e4f6a8578c4e3c154b Mon Sep 17 00:00:00 2001 From: Finn Bear Date: Sun, 28 Aug 2022 18:47:33 -0700 Subject: [PATCH] Refactor, optimization, and cleanup of code (#71) --- lib/src/fnc/rand.rs | 22 +++++---------- lib/src/fnc/string.rs | 51 ++++++++++++++++++---------------- lib/src/fnc/util/string/mod.rs | 4 +-- lib/src/sql/array.rs | 24 ++++++---------- lib/src/sql/duration.rs | 34 ++++++++++------------- lib/src/sql/regex.rs | 5 +--- lib/src/sql/value/del.rs | 10 ++++--- 7 files changed, 66 insertions(+), 84 deletions(-) diff --git a/lib/src/fnc/rand.rs b/lib/src/fnc/rand.rs index 26c7612e..0a4144a8 100644 --- a/lib/src/fnc/rand.rs +++ b/lib/src/fnc/rand.rs @@ -6,6 +6,7 @@ use crate::sql::uuid::Uuid; use crate::sql::value::Value; use nanoid::nanoid; use rand::distributions::Alphanumeric; +use rand::prelude::IteratorRandom; use rand::Rng; pub fn rand(_: &Context, _: Vec) -> Result { @@ -17,23 +18,14 @@ pub fn bool(_: &Context, _: Vec) -> Result { } pub fn r#enum(_: &Context, mut args: Vec) -> Result { - match args.len() { - 0 => Ok(Value::None), + Ok(match args.len() { + 0 => Value::None, 1 => match args.remove(0) { - Value::Array(mut v) => match v.len() { - 0 => Ok(Value::None), - n => { - let i = rand::thread_rng().gen_range(0..n); - Ok(v.remove(i)) - } - }, - v => Ok(v), + Value::Array(v) => v.into_iter().choose(&mut rand::thread_rng()).unwrap_or(Value::None), + v => v, }, - n => { - let i = rand::thread_rng().gen_range(0..n); - Ok(args.remove(i)) - } - } + _ => args.into_iter().choose(&mut rand::thread_rng()).unwrap(), + }) } pub fn float(_: &Context, mut args: Vec) -> Result { diff --git a/lib/src/fnc/string.rs b/lib/src/fnc/string.rs index 7080c2c3..28c7cf1b 100644 --- a/lib/src/fnc/string.rs +++ b/lib/src/fnc/string.rs @@ -7,16 +7,18 @@ pub fn concat(_: &Context, args: Vec) -> Result { Ok(args.into_iter().map(|x| x.as_string()).collect::>().concat().into()) } -pub fn ends_with(_: &Context, mut args: Vec) -> Result { - let val = args.remove(0).as_string(); - let chr = args.remove(0).as_string(); +pub fn ends_with(_: &Context, args: Vec) -> Result { + let args: [Value; 2] = args.try_into().unwrap(); + let [val, chr] = args.map(Value::as_string); Ok(val.ends_with(&chr).into()) } -pub fn join(_: &Context, mut args: Vec) -> Result { - let chr = args.remove(0).as_string(); - let val = args.into_iter().map(|x| x.as_string()); - let val = val.collect::>().join(&chr); +pub fn join(_: &Context, args: Vec) -> Result { + let mut args = args.into_iter().map(Value::as_string); + let chr = args.next().unwrap(); + // FIXME: Use intersperse to avoid intermediate allocation once stable + // https://github.com/rust-lang/rust/issues/79524 + let val = args.collect::>().join(&chr); Ok(val.into()) } @@ -30,16 +32,16 @@ pub fn lowercase(_: &Context, mut args: Vec) -> Result { Ok(args.remove(0).as_string().to_lowercase().into()) } -pub fn repeat(_: &Context, mut args: Vec) -> Result { - let val = args.remove(0).as_string(); - let num = args.remove(0).as_int() as usize; +pub fn repeat(_: &Context, args: Vec) -> Result { + let [val_arg, num_arg]: [Value; 2] = args.try_into().unwrap(); + let val = val_arg.as_string(); + let num = num_arg.as_int() as usize; Ok(val.repeat(num).into()) } -pub fn replace(_: &Context, mut args: Vec) -> Result { - let val = args.remove(0).as_string(); - let old = args.remove(0).as_string(); - let new = args.remove(0).as_string(); +pub fn replace(_: &Context, args: Vec) -> Result { + let args: [Value; 3] = args.try_into().unwrap(); + let [val, old, new] = args.map(Value::as_string); Ok(val.replace(&old, &new).into()) } @@ -47,10 +49,11 @@ pub fn reverse(_: &Context, mut args: Vec) -> Result { Ok(args.remove(0).as_string().chars().rev().collect::().into()) } -pub fn slice(_: &Context, mut args: Vec) -> Result { - let val = args.remove(0).as_string(); - let beg = args.remove(0).as_int() as usize; - let lim = args.remove(0).as_int() as usize; +pub fn slice(_: &Context, args: Vec) -> Result { + let [val_arg, beg_arg, lim_arg]: [Value; 3] = args.try_into().unwrap(); + let val = val_arg.as_string(); + let beg = beg_arg.as_int() as usize; + let lim = lim_arg.as_int() as usize; let val = val.chars().skip(beg).take(lim).collect::(); Ok(val.into()) } @@ -59,16 +62,16 @@ pub fn slug(_: &Context, mut args: Vec) -> Result { Ok(string::slug(&args.remove(0).as_string()).into()) } -pub fn split(_: &Context, mut args: Vec) -> Result { - let val = args.remove(0).as_string(); - let chr = args.remove(0).as_string(); +pub fn split(_: &Context, args: Vec) -> Result { + let args: [Value; 2] = args.try_into().unwrap(); + let [val, chr] = args.map(Value::as_string); let val = val.split(&chr).collect::>(); Ok(val.into()) } -pub fn starts_with(_: &Context, mut args: Vec) -> Result { - let val = args.remove(0).as_string(); - let chr = args.remove(0).as_string(); +pub fn starts_with(_: &Context, args: Vec) -> Result { + let args: [Value; 2] = args.try_into().unwrap(); + let [val, chr] = args.map(Value::as_string); Ok(val.starts_with(&chr).into()) } diff --git a/lib/src/fnc/util/string/mod.rs b/lib/src/fnc/util/string/mod.rs index cb48115e..5a6fd7f6 100644 --- a/lib/src/fnc/util/string/mod.rs +++ b/lib/src/fnc/util/string/mod.rs @@ -9,9 +9,9 @@ pub fn slug>(s: S) -> String { // Get a reference let s = s.as_ref(); // Convert unicode to ascii - let s = deunicode(s); + let mut s = deunicode(s); // Convert string to lowercase - let s = s.to_ascii_lowercase(); + s.make_ascii_lowercase(); // Replace any non-simple characters let s = SIMPLES.replace_all(s.as_ref(), "-"); // Replace any duplicated hyphens diff --git a/lib/src/sql/array.rs b/lib/src/sql/array.rs index 269b24e9..a5948535 100644 --- a/lib/src/sql/array.rs +++ b/lib/src/sql/array.rs @@ -206,22 +206,14 @@ impl Abolish for Vec { where F: FnMut(usize) -> bool, { - let len = self.len(); - let mut del = 0; - { - let v = &mut **self; - - for i in 0..len { - if f(i) { - del += 1; - } else if del > 0 { - v.swap(i - del, i); - } - } - } - if del > 0 { - self.truncate(len - del); - } + let mut i = 0; + // FIXME: use drain_filter once stabilized (https://github.com/rust-lang/rust/issues/43244) + // to avoid negation of the predicate return value. + self.retain(|_| { + let retain = !f(i); + i += 1; + retain + }); } } diff --git a/lib/src/sql/duration.rs b/lib/src/sql/duration.rs index 5671b73a..d6469ae2 100644 --- a/lib/src/sql/duration.rs +++ b/lib/src/sql/duration.rs @@ -30,10 +30,7 @@ impl From for Duration { impl From for Duration { fn from(s: String) -> Self { - match duration(s.as_ref()) { - Ok((_, v)) => v, - Err(_) => Duration::default(), - } + s.as_str().into() } } @@ -64,6 +61,12 @@ impl fmt::Display for Duration { // Split up the duration let secs = self.0.as_secs(); let nano = self.0.subsec_nanos(); + + // Ensure no empty output + if secs == 0 && nano == 0 { + return write!(f, "0ns"); + } + // Calculate the total years let year = secs / SECONDS_PER_YEAR; let secs = secs % SECONDS_PER_YEAR; @@ -79,36 +82,29 @@ impl fmt::Display for Duration { // Calculate the total mins let mins = secs / SECONDS_PER_MINUTE; let secs = secs % SECONDS_PER_MINUTE; - // Prepare the outpit - let mut o = Vec::with_capacity(7); // Write the different parts if year > 0 { - o.push(format!("{year}y")); + write!(f, "{year}y")?; } if week > 0 { - o.push(format!("{week}w")); + write!(f, "{week}w")?; } if days > 0 { - o.push(format!("{days}d")); + write!(f, "{days}d")?; } if hour > 0 { - o.push(format!("{hour}h")); + write!(f, "{hour}h")?; } if mins > 0 { - o.push(format!("{mins}m")); + write!(f, "{mins}m")?; } if secs > 0 { - o.push(format!("{secs}s")); + write!(f, "{secs}s")?; } if nano > 0 { - o.push(format!("{nano}ns")); + write!(f, "{nano}ns")?; } - // Ensure no empty output - if o.is_empty() { - o.push("0ns".to_string()); - } - // Concatenate together - write!(f, "{}", o.concat()) + Ok(()) } } diff --git a/lib/src/sql/regex.rs b/lib/src/sql/regex.rs index ef48ef40..397ca81f 100644 --- a/lib/src/sql/regex.rs +++ b/lib/src/sql/regex.rs @@ -32,10 +32,7 @@ impl fmt::Display for Regex { impl Regex { pub fn regex(&self) -> Option { - match regex::Regex::new(&self.0) { - Ok(v) => Some(v), - Err(_) => None, - } + regex::Regex::new(&self.0).ok() } } diff --git a/lib/src/sql/value/del.rs b/lib/src/sql/value/del.rs index 01f7ae56..963f5922 100644 --- a/lib/src/sql/value/del.rs +++ b/lib/src/sql/value/del.rs @@ -8,7 +8,7 @@ use crate::sql::part::Part; use crate::sql::value::Value; use async_recursion::async_recursion; use futures::future::try_join_all; -use std::collections::HashMap; +use std::collections::HashSet; impl Value { #[cfg_attr(feature = "parallel", async_recursion)] @@ -91,13 +91,15 @@ impl Value { }, Part::Where(w) => match path.len() { 1 => { - let mut m = HashMap::new(); + // TODO: If further optimization is desired, push indices to a vec, + // iterate in reverse, and call swap_remove + let mut m = HashSet::new(); for (i, v) in v.iter().enumerate() { if w.compute(ctx, opt, txn, Some(v)).await?.is_truthy() { - m.insert(i, ()); + m.insert(i); }; } - v.abolish(|i| m.contains_key(&i)); + v.abolish(|i| m.contains(&i)); Ok(()) } _ => {