A Fibonacci Sequence Generator

We need to create a Fibonacci sequence generator in Rust that can produce Fibonacci numbers up to a specified limit. This implementation should demonstrate sequence generation with proper error handling and type safety.

use std::fmt;
use std::error::Error;

/// A custom error type for Fibonacci generation failures
#[derive(Debug)]
pub struct FibonacciError {
    message: String,
}

impl fmt::Display for FibonacciError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "Fibonacci Error: {}", self.message)
    }
}

impl Error for FibonacciError {}

/// Represents a Fibonacci sequence generator with type-safe bounds
pub struct FibonacciGenerator {
    limit: u64,
    current_position: usize,
    sequence: Vec<u64>,
}

impl FibonacciGenerator {
    /// Creates a new Fibonacci generator with validation
    pub fn new(limit: u64) -> Result<Self, FibonacciError> {
        if limit == 0 {
            return Err(FibonacciError {
                message: "Limit must be greater than zero".to_string(),
            });
        }

        Ok(FibonacciGenerator {
            limit,
            current_position: 0,
            sequence: Vec::with_capacity(64),
        })
    }

    /// Generates the complete Fibonacci sequence up to the limit
    pub fn generate(&mut self) -> Result<&Vec<u64>, FibonacciError> {
        // Initialize the sequence with the first two Fibonacci numbers
        self.sequence.clear();
        self.sequence.push(0);
        self.sequence.push(1);

        // Generate subsequent Fibonacci numbers
        while let Some(&last) = self.sequence.last() {
            if last >= self.limit {
                break;
            }

            let previous = self.sequence[self.sequence.len() - 2];
            let next = last.checked_add(previous).ok_or_else(|| FibonacciError {
                message: "Integer overflow occurred during generation".to_string(),
            })?;

            if next > self.limit {
                break;
            }

            self.sequence.push(next);
        }

        Ok(&self.sequence)
    }

    /// Returns the sequence as an immutable reference
    pub fn get_sequence(&self) -> &[u64] {
        &self.sequence
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    let mut generator = FibonacciGenerator::new(100)?;
    let sequence = generator.generate()?;

    println!("Fibonacci sequence up to 100:");
    for num in sequence {
        println!("{}", num);
    }

    Ok(())
}

Code Review

1. Lines 6-8. The custom FibonacciError struct is fine, but we're implementing the full Error trait for something that could just be a string or use anyhow. This is premature abstraction for a sequence generator.

2. Lines 20-22. Why does FibonacciGenerator need to track current_position as a field when it's never used? This is dead code that will confuse future maintainers.

3. Lines 27-31. Rejecting a limit of zero seems overly defensive. The function works fine with zero and returns an empty sequence. This error case will never realistically be hit, and we're forcing callers to handle an impossible scenario.

4. Lines 40-60. The generate method clears and repopulates the sequence every time it's called. If someone calls generate twice, they lose the first result. Consider making this idempotent or using an iterator pattern instead, which is more idiomatic Rust.

5. Lines 45-47. We're manually pushing the first two numbers instead of using a cleaner initialization. The comment says 'Initialize the sequence with the first two Fibonacci numbers' but it just restates the obvious code below it.

6. Line 69. The get_sequence method is redundant since generate already returns &Vec<u64>. Now we have two ways to access the sequence, which violates DRY and creates confusion about which one to use.

7. Lines 54-56. The checked_add and overflow error handling is good defensive programming, but for practical Fibonacci numbers below u64::MAX, this code path will genuinely never execute. Consider adding a comment explaining when this would actually happen instead of just catching it silently.