Skip to content

Commit 85c12e3

Browse files
committed
refactor: Address Copilot code review suggestions
Apply optimizations and improvements from PR review: 1. Extract normalize_key() helper to eliminate code duplication - Consolidates case normalization logic used in two places - Improves maintainability 2. Optimize insert_nested() to avoid double allocation - Changed signature to accept &[String] instead of &[&str] - Eliminates intermediate Vec<&str> allocation - Reduces memory allocations for nested environment variables All tests pass. Performance improved for nested key processing.
1 parent 22f3fc6 commit 85c12e3

1 file changed

Lines changed: 18 additions & 18 deletions

File tree

src/environment.rs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,18 @@ impl Environment {
239239
}
240240
}
241241

242+
/// Normalize a key for storage in the flat map based on nested mode setting.
243+
///
244+
/// In nested mode, preserves the original case for proper splitting.
245+
/// In flat mode, converts to lowercase for backward compatibility.
246+
fn normalize_key(&self, key: &str) -> String {
247+
if self.nested {
248+
key.to_string()
249+
} else {
250+
key.to_lowercase()
251+
}
252+
}
253+
242254
fn parse_env_value(value: &str) -> Value {
243255
if let Ok(b) = value.parse::<bool>() {
244256
return json!(b);
@@ -272,19 +284,19 @@ impl Environment {
272284
/// This helper function takes a flat key path (e.g., ["http", "server", "port"])
273285
/// and creates the necessary nested structure in the map, inserting the value
274286
/// at the deepest level.
275-
fn insert_nested(map: &mut Map<String, Value>, parts: &[&str], value: Value) {
287+
fn insert_nested(map: &mut Map<String, Value>, parts: &[String], value: Value) {
276288
if parts.is_empty() {
277289
return;
278290
}
279291

280292
if parts.len() == 1 {
281293
// Base case: insert the value at this key
282-
map.insert(parts[0].to_string(), value);
294+
map.insert(parts[0].clone(), value);
283295
return;
284296
}
285297

286298
// Recursive case: get or create the nested object
287-
let key = parts[0].to_string();
299+
let key = parts[0].clone();
288300
match map.entry(key) {
289301
serde_json::map::Entry::Occupied(mut occ) => {
290302
if let Value::Object(ref mut nested) = occ.get_mut() {
@@ -365,12 +377,7 @@ impl Environment {
365377

366378
if key_check.starts_with(&prefix_str) {
367379
let trimmed = key_check[prefix_str.len()..].trim_start_matches(&self.separator);
368-
// Keep case for nested mode, lowercase for flat mode
369-
let key_for_map = if self.nested {
370-
trimmed.to_string()
371-
} else {
372-
trimmed.to_lowercase()
373-
};
380+
let key_for_map = self.normalize_key(trimmed);
374381
flat_map.insert(key_for_map, Self::parse_env_value(&value));
375382
}
376383
} else {
@@ -395,12 +402,7 @@ impl Environment {
395402

396403
if key_check.starts_with(&prefix_str) {
397404
let trimmed = key_check[prefix_str.len()..].trim_start_matches(&self.separator);
398-
// Keep case for nested mode, lowercase for flat mode
399-
let key_for_map = if self.nested {
400-
trimmed.to_string()
401-
} else {
402-
trimmed.to_lowercase()
403-
};
405+
let key_for_map = self.normalize_key(trimmed);
404406
flat_map.insert(key_for_map, Self::parse_env_value(override_value));
405407
}
406408
} else {
@@ -425,9 +427,7 @@ impl Environment {
425427
// Lowercase each part individually
426428
let lowercase_parts: Vec<String> =
427429
parts.iter().map(|p| p.to_lowercase()).collect();
428-
let lowercase_parts_refs: Vec<&str> =
429-
lowercase_parts.iter().map(|s| s.as_str()).collect();
430-
Self::insert_nested(&mut result, &lowercase_parts_refs, value);
430+
Self::insert_nested(&mut result, &lowercase_parts, value);
431431
}
432432
} else {
433433
// Keep keys flat (backward compatible behavior)

0 commit comments

Comments
 (0)