Skip to content

Implement bubble sort in Ruby #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ArunSahadeo
Copy link
Contributor

No description provided.

@june128 june128 added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Jun 26, 2018
for i in 0..arr.length - 1
for k in 0..arr.length - 2
if arr[k] > arr[k + 1]
temp = arr[k + 1]
Copy link
Member

Choose a reason for hiding this comment

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

Using a, b = b, a notation here seems appropriate :)

@jiegillet
Copy link
Member

Yay, Ruby code :)
Welcome ArunSahadeo

@Butt4cak3
Copy link
Contributor

@jiegillet It looks like you know Ruby well enough to take this PR, so I assigned it to you.

@andreaswachowski
Copy link

andreaswachowski commented Jun 28, 2018

For the sake of consistency, I'd suggest to modify the code such that it is accepted by rubocop. Of course, this is a matter of preference and you might have intentionally abstained from using it. (I am also just passing by after finding this project via Youtube's 3Blue1Brown channel, so I don't know where this project stands with regard to Ruby code conventions.)

The parentheses for the main() function (ie a function without parameters) strike me as particularly unconventional in Ruby. Also, the return keyword may be omitted, and Rubocop suggests to prefer each over for. Eventually, including @jiegillet's suggestion, the result would be:

#!/usr/bin/env ruby

def bubble_sort(arr)
  (0..arr.length - 1).each do
    (0..arr.length - 2).each do |k|
      next unless arr[k] > arr[k + 1]
      arr[k + 1], arr[k] = arr[k], arr[k + 1]
    end
  end

  arr
end

def main
  range = [200, 79, 69, 45, 32, 5, 15, 88, 620, 125]
  puts "The range before sorting is #{range}"
  range = bubble_sort(range)
  puts "The range after sorting is #{range}"
end

main

Edit: Credit @jiegillet for the parallel assignment suggestion.

@jiegillet
Copy link
Member

Thanks for the input @andreaswachowski, I haven't written Ruby code in a while, so your suggestions are welcome. I also prefereach over for, although I'm not sure that next unless is better than a simple if in this case. No big deal though ^^

@@ -20,6 +20,8 @@ This means that we need to go through the vector $$\mathcal{O}(n^2)$$ times with
[import, lang:"haskell"](code/haskell/bubbleSort.hs)
{% sample lang="cpp" %}
[import, lang:"c_cpp"](code/c++/bubblesort.cpp)
{% sample lang="rb" %}
[import, lang:"ruby"](code/ruby/bubble.rb)
Copy link
Member

Choose a reason for hiding this comment

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

Let's skip the first two line, an merely import:3-7

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be import:3-12? So that the whole function is included.

Copy link
Member

Choose a reason for hiding this comment

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

Now that I look again, it's import:3-15 haha. Anyway, the line count will change after more commits, the point is to just import the function :)

@andreaswachowski
Copy link

@jiegillet I agree with you that the next is debatable. I followed rubocop's suggestion here, but as mentioned often that's a matter of preference.

@ArunSahadeo
Copy link
Contributor Author

@andreaswachowski I implemented your changes except for the next, which I did not think was necessary.

@jiegillet
Copy link
Member

Thank you! Unfortunately I can't even see your changes because you committed changes to 252 files :)
That's our fault though, the AAA has receive a surge of activity recently and a lot has changed, including the structure of the repo. If your git skills are up to it, you can pull the latest version of the AAA on your branch and clean it up. Otherwise, maybe you can start over a new PR.

@ArunSahadeo ArunSahadeo force-pushed the feature/add-bubble-sort-in-ruby branch from 8482334 to cf776d6 Compare July 26, 2018 20:48
@ArunSahadeo
Copy link
Contributor Author

Hey @jiegillet I made a forced update to my feature branch, should be up to date now (been busy at work haha)

Copy link
Member

@jiegillet jiegillet left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for your contribution!

@jiegillet jiegillet merged commit 6a4e99b into algorithm-archivists:master Jul 26, 2018
@ArunSahadeo ArunSahadeo deleted the feature/add-bubble-sort-in-ruby branch July 27, 2018 10:17
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.

5 participants