google / google/comprehensive-rust

Suggestions: Misc

Open
#70 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
33.4k
Forks
2.1k
Avg merge
1d 3h
Merged PRs (30d)
10

Description

- [ ] [I think you should show the `Option` and `Result` definitions here.](https://google.github.io/comprehensive-rust/std/option-result.html) They'll see the variants everywhere in Rust.

---

- [ ] [Clarify that a `HashMap` doesn't have to be keyed with a `&str` or `String`](https://google.github.io/comprehensive-rust/std/hashmap.html) (unlike a JSON object, say).

---

- [ ] [Here,](https://google.github.io/comprehensive-rust/ownership/lifetimes-function-calls.html) I was going to mention that the lifetime can be explicit in some cases... but upon reconsideration that's probably not worth getting into. However, perhaps you should mention that lifetimes with the same name must indeed be equal. This will matter a lot should they every run into some `&'a mut Thing<'a>` errors.

But also on that page [and on this one,](https://google.github.io/comprehensive-rust/ownership/lifetimes.html) you say
> Read &'a Point as “a borrowed Point which is valid for at least the lifetime a”.

That can be misinterpreted as "the borrow is valid for at least `'a`", as opposed to "the `Point` is valid for at least `'a`". I.e. it might given the mistaken impression that a borrow can be lengthened.

---

- [x] [The getters and setters here aren't very idiomatic.](https://google.github.io/comprehensive-rust/exercises/day-2/health-statistics.html) First of all, nothing here is public -- everything must be in the same module -- so you'd just access the fields directly. Second of all, instead of a getter an infallible setter, if the struct was public, you'd probably just make those fields public too. If the fields actually have invariants, the setters shouldn't be infallible, but instead return a `Result<_, _>`.

More generally, it may give the impression that using getters and setters is idiomatic (even in the same module).

---

- [x] [Perhaps note this list of receivers is non-exhaustive.](https://google.github.io/comprehensive-rust/methods/receiver.html) E.g. `Box`.

---

- [ ] [I don't know that "omitted" is the right term for `mod in_another_file;`.](https://google.github.io/comprehensive-rust/modules/filesystem.html) Also perhaps note that (except perhaps for `test` modules) separate files is the norm.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.