ruby sort_by! for array of objects

Question

Okay, I KNOW I'm just doing something dumb here, but I just cannot get this to work.

I have an opportunity object

class Opportunity
  attr_accessor :effort
  attr_accessor :value

  def initialize(effort,value)
    # set values to 1 + the Rayleigh distribution (rounded) 
    @effort = 1 + (effort * Math.sqrt(0-2*Math.log(1-rand()))).round
    @value = 1 + (value * Math.sqrt(0-2*Math.log(1-rand()))).round
  end
end

I push a set of these onto an array (that's a property of another object) and then I want to sort those by specific properties (like effort)

    # Order the working backlog
    #  (see https://ruby-doc.org/core-2.4.3/Array.html#method-i-sort_by-21)
    puts workingBacklog.backlog[0].effort
    workingBacklog.backlog.sort_by! {|opA,opB| opA.effort <=> opB.effort }

Here's sample output...

6
3cmc.rb:57:in `block (2 levels) in <main>': undefined method `effort' for 
nil:NilClass (NoMethodError)
    from 3cmc.rb:57:in `each'
    from 3cmc.rb:57:in `sort_by'
    from 3cmc.rb:57:in `sort_by!'
    from 3cmc.rb:57:in `block in <main>'
    from 3cmc.rb:51:in `each'
    from 3cmc.rb:51:in `<main>'

So I KNOW there's an array of non-NULL objects there because that "6" on the first line of output is non-Null. But right after that it looks like the proverbial wheels come off. What the heck is going on here?

tadman · Accepted Answer · 2018-10-30 19:35:05Z

5

The sort method requires a comparison block (a vs. b) but the sort_by method only takes one argument, the element being sorted, and you're expected to return a transformed version if necessary, the way you want it sorted.

In your case it's this:

backlog.sort_by! { |e| e.effort }

Or more concisely:

backlog.sort_by!(&:effort)

Your original code would work if you use the correct method:

backlog.sort! {|opA,opB| opA.effort <=> opB.effort }

Where that's a lot more code than the sort_by alternative, but it does the job just the same. Generally sort_by performs better, it only transforms once per sort pass. The other function must transform once per comparison, of which there are usually a lot more than entries in an array, especially at larger array sizes.

NOTE: In Ruby capital letters have significant contextual meaning, so variable and method names should be in lower_case only form. Capitals are reserved for ClassName and CONSTANT_NAME situations.

answered Oct 30, 2018 at 19:35

tadman

212k23 gold badges237 silver badges266 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Bruce P. Henry Over a year ago

Yup! I got the wrong method on the array. I knew this was dumb.

Bruce P. Henry Over a year ago

For the record, I'm planning on building MUCH more complex comparison functions for sorting the array. But I wanted to get a SUPER simple version working first before I start layering on complexity.

tadman Over a year ago

That's fair. Just keep in mind that sort_by operates N times and sort operates on average N x log(N) times, which can be a lot more. In almost all cases sort_by is the way to go. It's when you need to write very complex tie-breaking rules that the other comes into play, but if you can express each entry as a sortable value, which can include arrays (e.g. [ a.effort, a.tie_breaker ]) then sort_by can do the job.

tadman Over a year ago

Remember when writing a sort function that if a sorts before b then b must sort after a. This sounds obvious, but it's not hard to introduce rock-paper-scissors type loops in your logic where rock,paper,scissors and scissors,paper,rock are both valid sort orders. That leads to inconsistent output.

Schwern · Accepted Answer · 2018-10-30 19:43:45Z

First, fix your sort_by as per @tadman's answer.

backlog.sort_by!(&:effort)

So I KNOW there's an array of non-NULL objects there because that "6" on the first line of output is non-Null.

You only checked the first element of workingBacklog.backlog. There could still be a nil somewhere further in your backlog array.

Look for nil with a select.with_index.

backlog.select.with_index { |obj,idx|
  puts "nil at #{idx}" if obj == nil
}

Here's an example that also reproduces the error.

backlog = [Opportunity.new(3, 4), nil]
puts backlog[0].effort
backlog.sort_by!(&:effort)

4    
/Users/schwern/tmp/test.rb:16:in `each': undefined method `effort' for nil:NilClass (NoMethodError)
    from /Users/schwern/tmp/test.rb:16:in `sort_by'
    from /Users/schwern/tmp/test.rb:16:in `sort_by!'
    from /Users/schwern/tmp/test.rb:16:in `<main>'

Collectives™ on Stack Overflow

ruby sort_by! for array of objects

2 Answers 2

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related