This isn't a really a question about this specific case, but it appeared as I was going to ask about something related -- so I'll use it as an illustration.
I got some feedback on this: Option<ControlFlow<(), ()>>.
This is returned from a function which can signal three states to the caller:
None - Nothing to do, just wait for next event.
Some(ControlFlow::Continue(())) - Process the event and then keep the event loop going.
Some(ControlFlow::Break(())) - Terminate the event loop.
The reviewer argued that this should be replaced with something along the line of:
enum HandlerResult {
Wait,
Process,
Terminate
}
The argument for this was that it much more clearly conveys intent.
I will immediately concede that the reviewer has a point, because I had to explain how the Option<ControlFlow<(), ()>> should be decoded. On the other hand, even with a custom enum one often has to check the documentation to get the full context for each variant, and if one anyway needs to read the docs, then I'd argue it doesn't really matter much if one uses a custom enum or composes std types.
I know that one of my flaws is that I prefer to use std types to a degree that is probably not main-stream. My thinking is that std's types have a bunch of useful methods, and it's nice to get those for free. Also, it's nice to use types that aren't behind (long) paths (I chose a bad example, because ControlFlow is not in std's prelude). And somewhere in the back of my mind, I keep thinking that maybe some day the compiler may introduce some optimizations for the composed std types, that aren't easily accessible for custom types. (With that said, I know there's an aspiration, at least by some who work in the Rust factory, that std should not be special).
Sometimes I admittedly may shoehorn in std types where even I see that there's a pretty good case for defining a custom type -- in this case though I don't think I have. But I'm curious to know where on the spectrum other Rust developers fall: Custom enum or composing std types.
I tend to agree with your reviewer, that a custom enum would be better here, and that Option<ControlFlow<(), ()>> should be avoided unless it has a specific use — for example, perhaps you unwrap the Option and pass the ControlFlow to functions that accept only ControlFlow.
That said, neither implementation is unreasonable. I just have a general principle: when possible, favor boring, straightforward code.
It matters that Option<ControlFlow<(), ()>> is a composition of highly general parts, and so in order to understand it, one has to assemble the whole meaning from three parts (Option, ControlFlow, and ()) and understand that there are three possible outcomes as a derived fact rather than one that is written out explicitly.
For the moment of reading docs and deciphering what some code means, there is indeed barely any difference between a custom enum and a composition of std types.
However, once the reader needs to remember what they just learned, meaningful names become an advantage. A name that precisely describes the meaning of an encoding is simply easier to recall, because you have to remember the meaning regardless, and a well-chosen name carries that meaning directly, while a type composition leaves you to reconstruct it.
I lean on the side of custom enums too. If you find you need to use the functions on the std type, you could always add a single into_xyz method in your enum.
It might be an interesting exercise, when migrating from the std type to a custom enum, to change all use sites to the into_xyz() method call, and then see whether the use sites are simplified by refactoring to use the custom enum names directly.
I have been convinced by this thread to stop making lego towers of std types where a custom enum would be clearer -- but your suggestion is a nice compromise between the two. I'll tinker with this idea.
Note that, with std tower, it's non-trivial to keep the same style for others adding to your code; they could choose core::task::Poll instead of Option as more semantically obvious.
This is the piece that pushes me to say that the nested version isn't great, because it's not obvious to me that None is wait as opposed to skip or something else.
If anything, I'd have expected ControlFlow<(), Result<(), Wait>> (with struct Wait;) or something because you're continuing the event loop in the wait case, there's just not an event this time.
(I often find that Option gets rough in combination with things -- Option<Foo<Option<_>>> in particular means there's too many meanings for None.)
I would say nested Options have their valid uses. Let's say, you have a List of things that may or may not be there, so it fits Vec<Option<T>>. Then you take the Iter<Option<T>>, and its next method returns Option<Option<T>>. That may look a bit hairy first, but it's very clear that None means we have exhausted the List, and Some(None) means that particular thing isn't there, but we continue to look for things.
But notably that only happened in the combination of other things.
I agree sometimes with generics you end up with types that you'd never write otherwise, just like how you often get &&T in filter's closure.
That's different from if you write a fresh, unconstrained thing where you'd never have a &&u32 parameter and, similarly, I'd say you should avoid Option<Option<T>> in such functions.