Skip to content

Commit

Permalink
feat: remove unnecessary clones
Browse files Browse the repository at this point in the history
Remove unnecessary clones when calling get_key
  • Loading branch information
lennartkloock committed Jul 7, 2023
1 parent 96c70e4 commit 2701bfb
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 16 deletions.
4 changes: 2 additions & 2 deletions config/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub enum ParseError {
}

pub trait Source<C: Config> {
fn get_key(&self, path: KeyPath) -> Result<Option<Value>>;
fn get_key(&self, path: &KeyPath) -> Result<Option<Value>>;
}

/// You don't need to implemented this trait manually.
Expand Down Expand Up @@ -186,7 +186,7 @@ impl<C: Config> ConfigBuilder<C> {
Ok(self
.sources
.iter()
.map(|s| s.get_key(key_path.clone()))
.map(|s| s.get_key(&key_path))
.collect::<Result<Vec<_>>>()?
.into_iter()
.flatten()
Expand Down
2 changes: 1 addition & 1 deletion config/config/src/sources/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ impl<C: Config> CliSource<C> {
}

impl<C: Config> Source<C> for CliSource<C> {
fn get_key(&self, path: KeyPath) -> Result<Option<Value>> {
fn get_key(&self, path: &KeyPath) -> Result<Option<Value>> {
utils::get_key(&self.value, path)
}
}
2 changes: 1 addition & 1 deletion config/config/src/sources/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ fn parse_to_value(key_type: KeyType, s: &str) -> Result<Value> {
}

impl<C: Config> Source<C> for EnvSource<C> {
fn get_key(&self, path: KeyPath) -> Result<Option<Value>> {
fn get_key(&self, path: &KeyPath) -> Result<Option<Value>> {
utils::get_key(&self.value, path)
}
}
2 changes: 1 addition & 1 deletion config/config/src/sources/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl<C: Config> FileSource<C> {
}

impl<C: Config> Source<C> for FileSource<C> {
fn get_key(&self, path: KeyPath) -> Result<Option<Value>> {
fn get_key(&self, path: &KeyPath) -> Result<Option<Value>> {
utils::get_key(&self.content, path)
}
}
6 changes: 3 additions & 3 deletions config/config/src/sources/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ use serde_value::Value;

use crate::{KeyPath, Result};

pub fn get_key(mut current: &Value, path: KeyPath) -> Result<Option<Value>> {
for segment in path.clone() {
pub fn get_key(mut current: &Value, path: &KeyPath) -> Result<Option<Value>> {
for segment in path {
let Value::Map(map) = current else {
// Trying to access a field on a non-map type
// I'm not sure if we should return an error here
panic!("Trying to access a field on a non-map type: {}, {:#?}", path, current);
};
let Some(value) = map.get(&Value::String(segment)) else {
let Some(value) = map.get(&Value::String(segment.clone())) else {
return Ok(None);
};

Expand Down
16 changes: 8 additions & 8 deletions config/config/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn env() {
std::env::set_var("SCUF_ENABLED", "true");
let config = sources::EnvSource::<DummyConfig>::with_prefix("SCUF").unwrap();
assert_eq!(
config.get_key("enabled".into()).unwrap().unwrap(),
config.get_key(&"enabled".into()).unwrap().unwrap(),
Value::Bool(true),
);

Expand All @@ -65,7 +65,7 @@ fn env() {
std::env::set_var("LOGGING__JSON", "false");
let config = sources::EnvSource::<DummyConfig>::with_joiner(None, "__").unwrap();
assert_eq!(
config.get_key("logging.json".into()).unwrap().unwrap(),
config.get_key(&"logging.json".into()).unwrap().unwrap(),
Value::Bool(false),
);

Expand All @@ -74,7 +74,7 @@ fn env() {
std::env::set_var("LOGGING_JSON", "true");
let config = sources::EnvSource::<DummyConfig>::new().unwrap();
assert_eq!(
config.get_key("logging.json".into()).unwrap().unwrap(),
config.get_key(&"logging.json".into()).unwrap().unwrap(),
Value::Bool(true),
);
}
Expand All @@ -87,10 +87,10 @@ fn file() {
"#;
let config = sources::FileSource::<DummyConfig>::toml(data).unwrap();
assert_eq!(
config.get_key("test.key".into()).unwrap().unwrap(),
config.get_key(&"test.key".into()).unwrap().unwrap(),
Value::String("test_value".to_string())
);
assert_eq!(config.get_key("test.not_defined".into()).unwrap(), None);
assert_eq!(config.get_key(&"test.not_defined".into()).unwrap(), None);
}

#[test]
Expand All @@ -108,15 +108,15 @@ fn cli() {
]);
let cli = sources::CliSource::<DummyConfig>::with_matches(matches).unwrap();
assert_eq!(
cli.get_key("enabled".into()).unwrap().unwrap(),
cli.get_key(&"enabled".into()).unwrap().unwrap(),
Value::Bool(true),
);
assert_eq!(
cli.get_key("logging.level".into()).unwrap().unwrap(),
cli.get_key(&"logging.level".into()).unwrap().unwrap(),
Value::String("INFO".to_string()),
);
assert_eq!(
cli.get_key("logging.json".into()).unwrap().unwrap(),
cli.get_key(&"logging.json".into()).unwrap().unwrap(),
Value::Bool(false),
);
}
Expand Down

0 comments on commit 2701bfb

Please sign in to comment.