Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

The zip solution however requires any reviewer to look up on what it actually does, whereas the "if let" is more of a language fundamental and known to most reviewers.

Therefore I would actually prefer the long/verbose form without the zip.



I think `Option::zip(&username, &password)` makes it somewhat clearer what it does.


It's just the opposite. zip is a normal function written in normal code, so if the reviewer doesn't know what it does then they can just click through to the definition in their IDE. Whereas "if let" is some kind of special language construct that a reviewer has to look up through some special alternative channel if they don't understand it.


If I'm doing a code review I don't have an idea. I only see text (yeah - limitation of the tooling, but reality). I can search for the functions, but it's a hassle and I want to limit having to do it to as much as possible. It's even not super easy in Rust, since some functions are defined on (extension) traits and you don't exactly know where to search for them if you don't have the IDE support and are not already an expert.

"if let" is certainly a special construct - but it's also one that Rust authors and reviewers will typically encounter rather fast since a lot of error handling and Option unwrapping uses it. Knowing the majority of library functions will imho take longer.


Understanding - or looking up - library functions is something you're always going to have to do during code review (it's well worth getting IDE integration set up). zip is a very standard and well-known function (I used it yesterday, in a non-Rust codebase); it may well end up being better-known than "if let". Learning what a library function does is certainly never harder than learning what a keyword does and it's often easier (apart from anything else, you know that a library function follows the normal rules for functions, so if you can see how its output is used then you can often tell what it does. Whereas you can't rely on that kind of reasoning with language keywords).




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: