readability...

if !rooms
.get(&name)
.map(|turf| if let Turf::Mine = turf { true } else { false })
.unwrap_or_default()

After reading this I had the feeling I just finished a sudoku.

if let Some(room) = rooms.get(&name) && !room.is_mine() {

//...

}

At this point you have to actually be trolling

@jestdotty Maybe I misunderstand but I don't think the code you posted does more than what mine does

It's just an if (with missing body) with a combinator chain which evals to a bool, it doesn't initialize as far as I can see

@jestdotty > now you gotta also write .is_mine()!

Don't be so allergic to abstractions!

@12bitfloat sorry I was saying it's being used as a initializer

in mine if there's no room the if will pass as true

in yours if there's no room the if fails as false

matters not I guess though. I'm too bad at making sense of anything for whatever reason

@jestdotty If there's no room, your example should evaluate to the default value of bool which is false.

Before let chains, I'd've done something like this:

if !rooms.get(&name).map_or(false, |turf| matches!(turf, Turf::Mine))

@lorentz The boolean negation refers to the whole expression so the default gets negated too

I think she's right on the logic... But come on, there has to be a simpler way to write this

@12bitfloat I didn't notice the damn !

So apparently I instinctively added the top-level negation without noticing that it's actually there, but I think it demonstrates another advantage of the form with an explicit boolean value that I wrote; you can lower the negation into the expression to avoid negating a boolean that you created right here (whether as a literal or via Default). The resulting statement would look something like this:

if rooms.get(&name).map_or(true, |turf| !matches!(turf, Turf::Mine))

You may prefer to think of the entire expression under the negation as a unit with a distinct semantic meaning corresponding to a positive statement ("$name is a mine"), which would make top-level negation preferable, but I think in that case it's also a good idea to pull it out into a variable. Generally speaking, an expression should represent one concept. Sometimes that concept is inherently complicated to express with the tools available so it gets long, this is especially often the case with iterators, but if it has sub-concepts you want to keep intact, those should be extracted if possible.

idk maybe I'm smartassing over what's really a pretty trivial line by Rust standards. I write, edit, and work around way worse lines than your original every minute and they are not an arbiter of productivity. It's all fine.

Why is there not a find function... converting a whole collection to booleans seems inefficient...

Also I think @lorentz's if the only reasonably readable one

@BordedDev that "map" is probably Option::map or Result::map, the original collection is only being queried here.

@lorentz sooo........ it's a miss-named function? (and it's a zero or one item collection?)

@BordedDev Yeah, option (maybe in Haskell parlance, nullable elsewhere) is a 0..1 arity collection. Rust consistently calls this function `map`; It isn't part of a trait, but it actually corresponds to Haskell's fmap which is defined by Functor. It's used to transform (map) all of the values in a collection without affecting its structure.

@lorentz that's why I'm saying it should be a "find" function, since mapping a whole collection to check if a single condition is true seems highly inefficient.

@BordedDev well, the collection in this case is literally 1 byte larger than its potential element, so mapping it is pretty fast actually. But there's also Option::is_some_and and Option::is_none_or which are a lot more descriptive.

@lorentz I get that, but then it still needs to unwrap (I assume the compiler will optimise it), but yeah the is_XXX is what I mean with find :D

@BordedDev I'm completely certain that the compiler will inline both the map and the unwrap_or_default, recognize that we're immediately branching on the result, remove the construction of the short-lived Option<bool>, and finally produce the same exact assembly for both lines. This is the exact kind of thing LLVM was made for.

rooms.get(&name).map_or(false, |turf| matches!(turf, Turf::Mine))

but omg I hate method chaining. It looks like a hold over from data processing days and making really bad assumptions about everything never failing.

I guess it is OK for very high level languages our script languages where the programmer is far removed from the hardware such that no real memory management is performed... but still I get the 😬😱😖 every time I see it.

@xcodesucks I prefer that of the local variable spam some like to do, especially in languages that support ?. or have null as a concrete type

Actually, wouldn't this just be:

if !matches!(rooms.get(&name), Some(Turf::Mine)) {}

@12bitfloat I feel so dumb now

@xcodesucks I think they call the pattern railway programming, routing a number of exit cases into the same codepath while transforming a value in the long path via operations that aren't otherwise related to each other is a useful pattern, and that's what Rust does, or tries to, with chaining and the ? operator. For it to be perfect we would need FromResidual to be stabilized, but even now it's very often the perfect abstraction, in the sense that if it didn't exist I would write the exact same code with no additional information but just less direct syntax.

(I also think Rust could benefit massively from either UFCS or a pipeline operator because the distinction between member functions and freestanding functions is largely arbitrary and leads to stupid workarounds like extension traits that contribute nothing but a band-aid fix and confusion.)

@lorentz Rust does have UFCS, but yeah a better pipelining and currying/binding operator would be really nice