Refactoring and Best Practices

Now that we have a working, tested program, let's improve it with better design patterns and performance optimizations.

Current Implementation Review

Our current search function works but has some inefficiencies:

public fn search(config: &Config, contents: &str): Vec<String> {
    var results = Vec.new()

    let query = if config.ignoreCase {
        config.query.toLowercase()
    } else {
        config.query.clone()
    }

    for line in contents.lines() {
        let searchLine = if config.ignoreCase {
            line.toLowercase()
        } else {
            line.toString()
        }

        if searchLine.contains(&query) {
            results.push(line.toString())
        }
    }

    results
}

Issues:

  1. Allocates query multiple times - We prepare the query inside the function
  2. Converts each line - For case-insensitive search, we convert every line
  3. Collects all results - For large files, this uses a lot of memory
  4. String cloning - Unnecessary cloning of lines

Refactoring: Pre-Process the Query

Prepare the query once:

public struct SearchConfig {
    public query: String,
    public originalQuery: String,
    public ignoreCase: Bool,
}

extension SearchConfig {
    public static fn new(query: String, ignoreCase: Bool): SearchConfig {
        let searchQuery = if ignoreCase {
            query.toLowercase()
        } else {
            query.clone()
        }

        SearchConfig {
            query: searchQuery,
            originalQuery: query,
            ignoreCase,
        }
    }

    public fn matches(line: &str): Bool {
        if self.ignoreCase {
            line.toLowercase().contains(&self.query)
        } else {
            line.contains(&self.query)
        }
    }
}

Refactoring: Using Iterators

Modern Rust/Oxide code uses iterators instead of manual loops. Here's a cleaner version:

public fn search(config: &Config, contents: &str): Vec<String> {
    let searchConfig = SearchConfig.new(
        config.query.clone(),
        config.ignoreCase
    )

    contents
        .lines()
        .filter { line -> searchConfig.matches(line) }
        .map { line -> line.toString() }
        .collect()
}

This is more idiomatic and easier to understand at a glance.

Refactoring: Streaming Results

For large files, collecting all results in memory is wasteful. Instead, stream results directly:

public fn search<F>(config: &Config, contents: &str, var callback: F)
where F: FnMut(&str) {
    let searchConfig = SearchConfig.new(
        config.query.clone(),
        config.ignoreCase
    )

    for line in contents.lines() {
        if searchConfig.matches(line) {
            callback(line)
        }
    }
}

public fn run(config: &Config): Result<(), Box<dyn Error>> {
    let contents = std.fs.readToString(&config.filename)?

    search(config, &contents) { line ->
        println!("{}", line)
    }

    Ok(())
}

Refactoring: Better Config Struct

Separate concerns in the Config struct:

public struct Config {
    public query: String,
    public filename: String,
    public searchOptions: SearchOptions,
}

public struct SearchOptions {
    public ignoreCase: Bool,
    public invertMatch: Bool,  // Show non-matching lines
    public lineNumbers: Bool,  // Show line numbers
    public countOnly: Bool,    // Just show count
}

extension SearchOptions {
    public static fn new(): SearchOptions {
        SearchOptions {
            ignoreCase: std.env.var("OXGREP_IGNORE_CASE").isOk(),
            invertMatch: false,
            lineNumbers: false,
            countOnly: false,
        }
    }
}

extension Config {
    public static fn new(args: &Vec<String>): Result<Config, String> {
        if args.len() < 3 {
            return Err("not enough arguments".toString())
        }

        let query = args[1].clone()
        let filename = args[2].clone()
        var searchOptions = SearchOptions.new()

        for i in 3..args.len() {
            match args[i].asStr() {
                "--ignore-case" -> searchOptions.ignoreCase = true,
                "--invert-match" -> searchOptions.invertMatch = true,
                "--line-numbers" -> searchOptions.lineNumbers = true,
                "--count" -> searchOptions.countOnly = true,
                flag -> return Err(format!("Unknown flag: {}", flag)),
            }
        }

        Ok(Config {
            query,
            filename,
            searchOptions,
        })
    }
}

Refactoring: Add Features Incrementally

Now adding new features is easier. Here's case-insensitive search:

extension SearchConfig {
    public fn matches(line: &str): Bool {
        let lineToSearch = if self.ignoreCase {
            line.toLowercase()
        } else {
            line.toString()
        }

        if self.invertMatch {
            !lineToSearch.contains(&self.query)
        } else {
            lineToSearch.contains(&self.query)
        }
    }
}

And line numbers:

public fn searchWithLineNumbers(
    config: &Config,
    contents: &str
): Vec<(UIntSize, String)> {
    let searchConfig = SearchConfig.new(
        config.query.clone(),
        config.searchOptions.ignoreCase
    )

    contents
        .lines()
        .enumerate()
        .filter { (_, line) -> searchConfig.matches(line) }
        .map { (num, line) -> (num + 1, line.toString()) }
        .collect()
}

Refactoring: Error Handling

Use custom error types for better error handling:

import std.fmt
import std.error.Error as StdError
import std.io

#[derive(Debug)]
public enum AppError {
    IoError(std.io.Error),
    ConfigError(String),
}

extension AppError: fmt.Display {
    fn fmt(f: &mut fmt.Formatter): fmt.Result {
        match self {
            AppError.IoError(err) -> write!(f, "IO error: {}", err),
            AppError.ConfigError(msg) -> write!(f, "Config error: {}", msg),
        }
    }
}

extension AppError: StdError {}

extension AppError: From<std.io.Error> {
    static fn from(err: std.io.Error): Self {
        AppError.IoError(err)
    }
}

extension AppError: From<String> {
    static fn from(err: String): Self {
        AppError.ConfigError(err)
    }
}

public fn run(config: &Config): Result<(), AppError> {
    let contents = std.fs.readToString(&config.filename)?

    search(config, &contents) { line ->
        println!("{}", line)
    }

    Ok(())
}

Refactoring: Documentation

Add documentation for public APIs:

/// Searches for a pattern in text, with optional case-insensitivity.
///
/// # Arguments
///
/// * `config` - Search configuration including the query and options
/// * `contents` - The text to search in
/// * `callback` - Function to call for each matching line
///
/// # Example
///
/// ```oxide
/// let config = Config {
///     query: "test".toString(),
///     filename: "file.txt".toString(),
///     searchOptions: SearchOptions.new(),
/// }
///
/// search(&config, "test\nno match\ntest again") { line ->
///     println!("{}", line)
/// }
/// ```
public fn search<F>(config: &Config, contents: &str, var callback: F)
where F: FnMut(&str) {
    // Implementation
}

Refactoring: Benchmarking

For production code, benchmark different approaches:

# Add to Cargo.toml
[[bench]]
name = "search"
harness = false

Create benches/search.rs:

fn benchmarkCaseSensitive() {
    let largeContent = "..." // Large file content
    let config = Config {
        query: "search".toString(),
        filename: "bench.txt".toString(),
        searchOptions: SearchOptions { ignoreCase: false, .. },
    }

    // Measure time
    search(&config, &largeContent) { }
}

Run benchmarks:

cargo bench

Final Refactored Code Structure

Here's the complete, refactored program:

// src/lib.ox
import std.fs
import std.env

public struct Config {
    public query: String,
    public filename: String,
    public searchOptions: SearchOptions,
}

public struct SearchOptions {
    public ignoreCase: Bool,
    public invertMatch: Bool,
}

public struct SearchConfig {
    public query: String,
    public ignoreCase: Bool,
}

extension Config {
    public static fn new(args: &Vec<String>): Result<Config, String> {
        // ... argument parsing
        Ok(Config { /* ... */ })
    }
}

extension SearchConfig {
    public static fn new(query: String, ignoreCase: Bool): SearchConfig {
        let query = if ignoreCase {
            query.toLowercase()
        } else {
            query
        }
        SearchConfig { query, ignoreCase }
    }

    public fn matches(line: &str): Bool {
        let line = if self.ignoreCase {
            line.toLowercase()
        } else {
            line.toString()
        }
        line.contains(&self.query)
    }
}

public fn search<F>(config: &Config, contents: &str, var callback: F)
where F: FnMut(&str) {
    let searchConfig = SearchConfig.new(
        config.query.clone(),
        config.searchOptions.ignoreCase
    )

    for line in contents.lines() {
        if searchConfig.matches(line) {
            callback(line)
        }
    }
}

public fn run(config: &Config): Result<(), Box<dyn std.error.Error>> {
    let contents = std.fs.readToString(&config.filename)?

    search(config, &contents) { line ->
        println!("{}", line)
    }

    Ok(())
}

Summary

Good refactoring practices:

  • Extract methods - Move complex logic into helper functions
  • Use iterators - They're idiomatic and often faster
  • Separate concerns - Each struct should have one responsibility
  • Document APIs - Use doc comments for public items
  • Add tests first - Tests ensure refactoring doesn't break things
  • Benchmark - Profile before optimizing
  • Version incrementally - Make small, focused changes

This completes our CLI project! You now have a well-structured, tested, and maintainable program that demonstrates core Oxide concepts.