book icon indicating copy to clipboard operation
book copied to clipboard

ch05-03 #methods-with-more-parameters: can_hold() method potentially contains a bug.

Open nspsck opened this issue 1 year ago • 0 comments
trafficstars

URL to the section(s) of the book with this problem:

https://doc.rust-lang.org/stable/book/ch05-03-method-syntax.html#methods-with-more-parameters

Description of the problem:

The can_hold method was implemented as follow:

fn can_hold(&self, other: &Rectangle) -> bool {
    self.width > other.width && self.height > other.height
}

In my opinion, whether a rectangle can hold the other not only depends on a simple check like this. The other might still fit into this rectangle if it's height is less than this rectangle's width, when exceeding the width of this rectangle. Aka. if you turn the other by 90 degrees, it might still fit into the first one.

To clarify:

let rect1 = {
    width: 20,
    height: 30,
}
let rect2 = {
    width: 25,
    height: 10,
}

rect1 can hold rect2, even the method can_hold() will evaluate false.

Suggested fix:

fn can_hold(&self, other: &Rectangle) -> bool {
    self.width > other.width && self.height > other.height
        || self.width > other.height && self.height > other.width
}

Sorry, it's late night for me. Please double check if this is correct. Thank you very much for spending time on this issue. best regards!

PS. As in the contribute.md clearly stated, that one should only make changes to src, since this is located in the listing, I assumed that a PR such is not wanted. And, thank you well much for creating this book! This is clearly the most well thought book I've every read, I love that the fact that all my spontaneous experimental thoughts has always been covered!

nspsck avatar Apr 26 '24 22:04 nspsck