Eliminate Code Duplication in generate()

[AbdullahNamespace]

🔨 Refactor: Eliminate Code Duplication in generate()

Priority: 🟡 Medium

Description

The generate() method contains significant code duplication (~80% similar code) for processing directories and files. This violates the DRY (Don't Repeat Yourself) principle and makes maintenance harder.

Current Problem

Duplicated Code Blocks

// Block 1: Processing Directories (Lines ~50-90)
for item in &self.config.directories {
    current_count += 1;
    let (path, condition) = match item {
        DirectoryItem::Simple(p) => (p.clone(), None),
        DirectoryItem::Complex(c) => (c.path.clone(), c.condition.clone()),
    };
let skip = if let Some(cond) = &condition {
    !evaluate_condition(cond, &self.context)?
} else {
    false
};

if skip {
    print_step(current_count, total_items as u32, "Skip Dir", &path, Status::Skip);
    skipped_dirs += 1;
    continue;
}

let full_path = self.output_dir.join(&path);
if !full_path.exists() {
    fs::create_dir_all(&full_path)?;
    print_step(current_count, total_items as u32, "Create Dir", &path, Status::Success);
    created_dirs += 1;
} else {
    print_step(current_count, total_items as u32, "Exists Dir", &path, Status::Skip);
    skipped_dirs += 1;
}

}
// Block 2: Processing Files (Lines ~95-140)

// ⚠️ ALMOST IDENTICAL CODE! Only differences:

// - "Dir" → "File"

// - fs::create_dir_all() → fs::write() / fs::File::create()

// - created_dirs → created_files

Impact

• Code Smell: DRY violation
• Maintenance Burden: Bug fixes need to be applied twice
• Test Coverage: Needs duplicate tests
• File Size: ~100 lines of duplicated logic
• Future Risk: Adding features requires changing 2+ places

Proposed Solution

Option 1: Generic Item Processor (Recommended)

// New trait for processable items
trait ProcessableItem {
    fn get_path(&self) -> String;
    fn get_condition(&self) -> Option<String>;
    fn item_type(&self) -> &'static str;
}
impl ProcessableItem for DirectoryItem {

fn get_path(&self) -> String {

match self {

DirectoryItem::Simple(p) => p.clone(),

DirectoryItem::Complex(c) => c.path.clone(),

}

}
fn get_condition(&amp;self) -&gt; Option&lt;String&gt; {
    match self {
        DirectoryItem::Simple(_) =&gt; None,
        DirectoryItem::Complex(c) =&gt; c.condition.clone(),
    }
}

fn item_type(&amp;self) -&gt; &amp;'static str {
    "Dir"
}

}
impl ProcessableItem for FileItem {

fn get_path(&self) -> String {

match self {

FileItem::Simple(p) => p.clone(),

FileItem::Complex(c) => c.path.clone(),

}

}
fn get_condition(&amp;self) -&gt; Option&lt;String&gt; {
    match self {
        FileItem::Simple(_) =&gt; None,
        FileItem::Complex(c) =&gt; c.condition.clone(),
    }
}

fn item_type(&amp;self) -&gt; &amp;'static str {
    "File"
}

}
// Generic processor

struct ItemProcessingStats {

created: u32,

skipped: u32,

}
impl ProjectGenerator {

fn process_items<T: ProcessableItem>(

&self,

items: &[T],

total_items: u32,

start_count: u32,

creator: impl Fn(&Path) -> Result<()>,

) -> Result<ItemProcessingStats> {

let mut stats = ItemProcessingStats {

created: 0,

skipped: 0,

};

let mut current_count = start_count;
    for item in items {
        current_count += 1;
        let path = item.get_path();
        let condition = item.get_condition();

        // Evaluate condition
        let skip = if let Some(cond) = &amp;condition {
            !evaluate_condition(cond, &amp;self.context)?
        } else {
            false
        };

        if skip {
            print_step(
                current_count,
                total_items,
                &amp;format!("Skip {}", item.item_type()),
                &amp;path,
                Status::Skip,
            );
            stats.skipped += 1;
            continue;
        }

        let full_path = self.output_dir.join(&amp;path);

        // Create item
        if !full_path.exists() {
            creator(&amp;full_path)?;
            print_step(
                current_count,
                total_items,
                &amp;format!("Create {}", item.item_type()),
                &amp;path,
                Status::Success,
            );
            stats.created += 1;
        } else {
            print_step(
                current_count,
                total_items,
                &amp;format!("Exists {}", item.item_type()),
                &amp;path,
                Status::Skip,
            );
            stats.skipped += 1;
        }
    }

    Ok(stats)
}

}

Usage in generate()

pub fn generate(&mut self, output_arg: Option<&str>) -> Result<GenerationStats> {
    // ... setup code ...
let total_items = self.config.directories.len() + self.config.files.len();

// Process directories
crate::ui::print_section("Creating Directories");
let dir_stats = self.process_items(
    &amp;self.config.directories,
    total_items as u32,
    0,
    |path| {
        fs::create_dir_all(path)?;
        Ok(())
    },
)?;

// Process files
crate::ui::print_section("Creating Files");
let file_stats = self.process_items(
    &amp;self.config.files,
    total_items as u32,
    self.config.directories.len() as u32,
    |path| {
        // Handle file content
        let file_item = /* get current file item */;
        match file_item {
            FileItem::Complex(c) if c.content.is_some() =&gt; {
                fs::write(path, c.content.as_ref().unwrap())?;
            }
            _ =&gt; {
                fs::File::create(path)?;
            }
        }
        Ok(())
    },
)?;

Ok(GenerationStats {
    // ... use stats ...
    dirs_count: dir_stats.created,
    files_count: file_stats.created,
    skipped_count: dir_stats.skipped + file_stats.skipped,
    // ...
})

}

Option 2: Extract Shared Logic Functions

impl ProjectGenerator {
    // Extract condition evaluation logic
    fn should_skip_item(&self, condition: &Option<String>) -> Result<bool> {
        if let Some(cond) = condition {
            Ok(!evaluate_condition(cond, &self.context)?)
        } else {
            Ok(false)
        }
    }
// Extract path extraction
fn extract_item_info&lt;T&gt;(&amp;self, item: &amp;T) -&gt; (String, Option&lt;String&gt;)
where
    T: /* trait for getting path and condition */
{
    // ...
}

// Extract creation logic
fn create_item_if_needed(
    &amp;self,
    path: &amp;str,
    current: u32,
    total: u32,
    item_type: &amp;str,
    creator: impl FnOnce(&amp;Path) -&gt; Result&lt;()&gt;,
) -&gt; Result&lt;bool&gt; {
    // Returns true if created, false if skipped/exists
    // ...
}

}

Benefits

Before (Current)

// 150+ lines in generate()
// Duplicated logic
// Hard to test individual pieces
// Changes require editing 2 places

After (Refactored)

// ~80 lines in generate()
// Reusable components
// Easy to test
// Single source of truth
// Extensible for new item types

Testing Strategy

#[cfg(test)]
mod tests {
    use super::*;
#[test]
fn test_process_items_with_conditions() {
    let gen = create_test_generator();
    let items = vec![
        DirectoryItem::Complex(ComplexItem {
            path: "test1".to_string(),
            condition: Some("$DOCKER == true".to_string()),
        }),
        DirectoryItem::Complex(ComplexItem {
            path: "test2".to_string(),
            condition: Some("$DOCKER == false".to_string()),
        }),
    ];

    let stats = gen.process_items(&items, 2, 0, |_| Ok(())).unwrap();
    assert_eq!(stats.created, 1);
    assert_eq!(stats.skipped, 1);
}

#[test]
fn test_process_items_all_created() {
    let gen = create_test_generator();
    let items = vec![
        DirectoryItem::Simple("src".to_string()),
        DirectoryItem::Simple("tests".to_string()),
    ];

    let stats = gen.process_items(&items, 2, 0, |_| Ok(())).unwrap();
    assert_eq!(stats.created, 2);
    assert_eq!(stats.skipped, 0);
}

}

Migration Plan

1. Step 1: Create new trait and implementations
2. Step 2: Implement process_items() function
3. Step 3: Add tests for new function
4. Step 4: Refactor generate() to use new function
5. Step 5: Remove old duplicated code
6. Step 6: Verify all existing tests still pass

Metrics

Metric	Before	After	Improvement
Lines of Code	~150	~80	-47%
Cyclomatic Complexity	12	6	-50%
Test Coverage Needed	2x	1x	-50%
Maintenance Points	2	1	-50%

Action Items

• Design trait hierarchy for ProcessableItem
• Implement process_items() generic function
• Write unit tests for new abstraction
• Refactor generate() to use new code
• Verify integration tests pass
• Update documentation if needed
• Remove old code
• Add benchmarks to ensure no performance regression

Notes

• This is pure refactoring - no behavior changes
• All existing tests should pass without modification
• Consider adding this to v1.1 roadmap
• Low risk, high reward change

Related

• Could enable easier implementation of new item types (symlinks, templates, etc.)
• Makes future features like parallel processing easier