Skip to content
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

FIX - Vector sort now sorts nil at the beginning #52

Merged
merged 2 commits into from
Feb 17, 2016

Conversation

gnilrets
Copy link
Contributor

No description provided.

@gnilrets
Copy link
Contributor Author

Resolves #39.

So this fixes the issue but I'm not sure it's the best solution. The problem is that if the user wanted to write their own sorter (by passing a block to sort), they'd still have to deal with the nil issue. An alternative to the solution in this commit might be to add refinements to many of the core classes (NilClass, Fixnum, Float, etc) that allowed them to be sortable when nils are involved.

class NilClass
  include Comparable
  def <=>(other)
    return 0 if other.nil?
    -1
  end
end

That, of course, comes with its own set of issues. Any thought on this?

@gnilrets
Copy link
Contributor Author

Also, it looks like my editor is removing trailing whitespace (which is good according to https://github.com/bbatsov/ruby-style-guide#no-trailing-whitespace).

@v0dro
Copy link
Member

v0dro commented Feb 16, 2016

Good fix. My only concern is speed. Will this slow things down significantly? The problem is that rewriting sorting in Ruby is slow as it is. The if conditions should not slow it down further.

@gnilrets
Copy link
Contributor Author

This commit should slightly reduce the number of evaluations that need to happen when your data doesn't have many nils. Not sure if we can do much better.

@v0dro
Copy link
Member

v0dro commented Feb 17, 2016

This will work for now. Thanks. Merging.

v0dro added a commit that referenced this pull request Feb 17, 2016
FIX - Vector sort now sorts nil at the beginning
@v0dro v0dro merged commit 9f25014 into SciRuby:master Feb 17, 2016
@gnilrets gnilrets deleted the FIX-NilSort branch April 18, 2016 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants