Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jun 27, 2018

Added a Rust implementation of bubble sort using the rand crate as found at https://crates.io/crates/rand.

If the random number generation for making a vector looks really esoteric, I'm willing to add another example with an easier-to-read sample vector!

@june128 june128 added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Jun 27, 2018
@Butt4cak3
Copy link
Contributor

I don't think we have many Rust people who can review PRs on a regular basis. Prepare for this one to last for a while. It will get merged eventually, though.

Copy link
Contributor

@zsparal zsparal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, I only have two small suggestions. Also @Butt4cak3, I can review Rust PRs :)

for _ in 0..n {
for j in 1..n {
if a[j - 1] > a[j] {
// Swaps values at indices j - 1 and j
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust has a swap method on slices, I would use that instead of the explicit swap written here. It's faster and it's more descriptive

use rand::{thread_rng, Rng}; // Used for random number generation
use rand::distributions::Uniform; // Used for a uniform distribution

fn bubble_sort(a: &mut Vec<u32>) {
Copy link
Contributor

@zsparal zsparal Jun 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should take &mut [u32] since it doesn't use any functionality on Vec, and taking slices is strictly more general (for example it could allow you to partially sort a vector).

if a[j - 1] > a[j] {
// Swaps values at indicies j - 1 and j
a.swap(j, j - 1);
a.swap(j, j - 1); // Swaps values at indicies j - 1 and j
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doSomething(); // does something

I'm sorry. I had to. :D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this comment is not necessary

@zsparal zsparal merged commit 4e3bb48 into algorithm-archivists:master Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants