Updated Binary Search docs to show insert into an already sorted list #61742
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
No problem! Thanks for the review help, grammar isn't my strong suit! |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Why something like this? if let Err(pos) = s.binary_search(&num) {
s.insert(pos, num);
} |
…ed list Resolves: rust-lang#61684 Apply suggestions from code review Co-Authored-By: Jonas Schievink <jonasschievink@gmail.com> Cleanup: Fixed failing tidy checks
|
was squashing cleanup commits into one commit, and @ondj I believe that would work as well, this way just seemed like it would allow more modification (editing of the OK() or Err() blocks) |
|
Ping @ptallo |
src/libcore/slice/mod.rs
Outdated
| /// let num = 14; | ||
| /// | ||
| /// match s.binary_search(&num) { | ||
| /// Ok(_) => {} |
There was a problem hiding this comment.
Hm, I think we should be inserting regardless of finding the item here, so filling out this Ok branch with an insert as well would be good, perhaps even illustrating that by doing something like Ok(idx) | Err(idx) => s.insert(idx, num) in an if let.
There was a problem hiding this comment.
I believe my new commit is more what you were looking for. Do I need to squash the commits or will the merge do that for me?
There was a problem hiding this comment.
You'll need to squash the commits; it also looks like you have a build failure here -- I suspect the ; after the if let.
|
I'll take over review here for now, left a comment. |
| /// let num = 14; | ||
| /// | ||
| /// let idx = s.binary_search(&num); | ||
| /// if let Ok(idx) | Err(idx) { |
There was a problem hiding this comment.
I believe this is invalid syntax and should be if let Ok(idx) | Err(idx) = idx {
There was a problem hiding this comment.
See error:
error: expected one of `=` or `|`, found `{`
--> slice/mod.rs:1361:27
|
8 | if let Ok(idx) | Err(idx) {
| ^ expected one of `=` or `|` here
| /// | ||
| /// let idx = s.binary_search(&num); | ||
| /// if let Ok(idx) | Err(idx) { | ||
| /// s.insert(idx, num) |
There was a problem hiding this comment.
There have been losed a semicolon
s.insert(idx,num);
Mark-Simulacrum
added
S-waiting-on-author
|
ping from triage, @ptallo any updates on this? |
rustbot
added
S-inactive
|
I rework in #63442. |
…k-Simulacrum Add an example to show how to insert item to a sorted vec Closes rust-lang#61684 cc rust-lang#61742 r? @Mark-Simulacrum, @jonas-schievink
…k-Simulacrum Add an example to show how to insert item to a sorted vec Closes rust-lang#61684 cc rust-lang#61742 r? @Mark-Simulacrum, @jonas-schievink
Resolves: #61684