comprehensive-rust icon indicating copy to clipboard operation
comprehensive-rust copied to clipboard

fix pattern matching exercise solution

Open mustafapc opened this issue 4 months ago • 3 comments

my solution works on more complex Expressions and Operations than the old one

mustafapc avatar Oct 18 '25 09:10 mustafapc

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Oct 18 '25 09:10 google-cla[bot]

Sorry, the change looks like a stylistic refactoring to me. What's the motivation?

gribozavr avatar Oct 18 '25 19:10 gribozavr

@gribozavr the motivation is that the solution used on the tutorial only works on trees that's two or three operation deep and mine can work with complex trees, try a operation for example ((((4 + 7) * (8 - 2)) / ((9 + 14) * (16 - 4))) + 5) or something even deeper than this my solution would work and the one in tutorial won't

mustafapc avatar Oct 18 '25 20:10 mustafapc

@mustafapc Sorry, I can't reproduce the problem yet. I translated the expression you provided into a test and it passes:

#[test]
fn test_complex_expression() {
    // ((((4 + 7) * (8 - 2)) / ((9 + 14) * (16 - 4))) + 5)
    let term1 = Expression::Op { // (4 + 7) * (8 - 2)
        op: Operation::Mul,
        left: Box::new(Expression::Op {
            op: Operation::Add,
            left: Box::new(Expression::Value(4)),
            right: Box::new(Expression::Value(7)),
        }),
        right: Box::new(Expression::Op {
            op: Operation::Sub,
            left: Box::new(Expression::Value(8)),
            right: Box::new(Expression::Value(2)),
        }),
    };
    let term2 = Expression::Op { // (9 + 14) * (16 - 4)
        op: Operation::Mul,
        left: Box::new(Expression::Op {
            op: Operation::Add,
            left: Box::new(Expression::Value(9)),
            right: Box::new(Expression::Value(14)),
        }),
        right: Box::new(Expression::Op {
            op: Operation::Sub,
            left: Box::new(Expression::Value(16)),
            right: Box::new(Expression::Value(4)),
        }),
    };
    let expr = Expression::Op { // term1 / term2 + 5
        op: Operation::Add,
        left: Box::new(Expression::Op {
            op: Operation::Div,
            left: Box::new(term1),
            right: Box::new(term2),
        }),
        right: Box::new(Expression::Value(5)),
    };
    assert_eq!(eval(expr), 5);
}

Could you provide a test that fails and demonstrates the bug?

gribozavr avatar Nov 15 '25 14:11 gribozavr

@mustafapc Sorry, I can't reproduce the problem yet. I translated the expression you provided into a test and it passes:

#[test]
fn test_complex_expression() {
    // ((((4 + 7) * (8 - 2)) / ((9 + 14) * (16 - 4))) + 5)
    let term1 = Expression::Op { // (4 + 7) * (8 - 2)
        op: Operation::Mul,
        left: Box::new(Expression::Op {
            op: Operation::Add,
            left: Box::new(Expression::Value(4)),
            right: Box::new(Expression::Value(7)),
        }),
        right: Box::new(Expression::Op {
            op: Operation::Sub,
            left: Box::new(Expression::Value(8)),
            right: Box::new(Expression::Value(2)),
        }),
    };
    let term2 = Expression::Op { // (9 + 14) * (16 - 4)
        op: Operation::Mul,
        left: Box::new(Expression::Op {
            op: Operation::Add,
            left: Box::new(Expression::Value(9)),
            right: Box::new(Expression::Value(14)),
        }),
        right: Box::new(Expression::Op {
            op: Operation::Sub,
            left: Box::new(Expression::Value(16)),
            right: Box::new(Expression::Value(4)),
        }),
    };
    let expr = Expression::Op { // term1 / term2 + 5
        op: Operation::Add,
        left: Box::new(Expression::Op {
            op: Operation::Div,
            left: Box::new(term1),
            right: Box::new(term2),
        }),
        right: Box::new(Expression::Value(5)),
    };
    assert_eq!(eval(expr), 5);
}

Could you provide a test that fails and demonstrates the bug?

I don't write rust anymore, have fun doing rust

mustafapc avatar Nov 15 '25 14:11 mustafapc