rust-unofficial / patterns

A catalogue of Rust design patterns, anti-patterns and idioms
https://rust-unofficial.github.io/patterns/
Mozilla Public License 2.0
7.84k stars 352 forks source link

Typo in idioms/coercion-arguments.md #377

Closed horw closed 3 months ago

horw commented 8 months ago

Hello, just small fix in

https://github.com/rust-unofficial/patterns/blob/79467dc4480fd93c85596d4a7bd18160527ce3e2/src/idioms/coercion-arguments.md?plain=1#L38C16-L38C38

vowel_count>=3 should be changed to vowel_count == 3 because reaching 4 is not possible.

neithernut commented 8 months ago

Disclaimer: not a maintainer

I don't think this is a typo. Yes, vowel_count will never be greater than 3, but I, too, would have chosen >= over == in this case, because I usually go full state coverage if given some condition like in this. With a ==, states with vowel_count < 3 are taken care of because we intend to continue to iterate and the state with vowel_count is also obviously taken care of, but all states with vowel_count > 3 are not.

Going for full state coverage is, at least in my experience, something you learn to appreciate if you do anything safety-related [^1]. It makes your stuff more resilient against all kinds of things, including random bitflips (e.g. in environments with higher radiation such as space) [^2] and human error committed by some developer [^3]. This may seem paranoid, but in these cases being paranoid doesn't hurt.

The >= doesn't even come with any extra cost. The HW instruction for comparing/conditional jump should be equally wide for == and >=, and you'll likely not being able to measure a significant difference in speed or power consumption either.

[^1]: and/or if you have written enough C [^2]: Granted, if you specifically wanted to maximize change of a correct result under the assumptions of bitflips only in vowel_count, you'd do a few calculations and likely end up with == after all. But in the general case full coverage is still preferable. [^3]: especially in projects involving C code

horw commented 8 months ago

Hi @neithernut, thanks for your review! You mentioned saved programming and its really important, but in this case what I see:

simonsan commented 8 months ago
  • it's a func that calls three_vowels, so it will be strange if there will be 4..etc, if we say about save programming it will have to raise error because it's unexpected behaviour

I don't agree, a word that contains 4 consecutive vowels does also contain 3 consecutive vowels. So you wouldn't ever expect a word with 4 consecutive vowels to get a false response from three_vowels, wouldn't you? I think this example is correct - for what it is, an example.

How someone would implement that in production, depends on a variety of different factors, for sure. And this implementation might not be well suited if you want to determine differences between 3 and 4 vowel words.