CODEWITHSUNDEEP
ruby-on-rails
Chiperific
Chiperific

Reputation: 4686

Rails controller - random record

Simple question, will this code ensure that the rand() call doesn't return the same record twice in a row?

It seems to, from click tests, but I really don't feel confident.

def thoughts  
  last_thought = 0 if last_thought == nil #this is the part that seems wrong for my goal
  th = Array.new

  Thought.all.each do |t|
    th << t.id
  end

  th.pop(last_thought)
  @rand_thought = Thought.find(rand(th.first..th.last))
  last_thought = @rand_thought.id
end

Based upon @user198201's suggestions, I've revised the code to this:

def thoughts
  shuffle_thought ||= Thought.all.shuffle
  @rand_thought = shuffle_thought.pop
  @rand_thought
end

Then to test, I added this method:

def thinker
  a = Array.new
  20.times do
    a << thoughts.id
  end
  a
end

Here's what I get now in the Rails Console:

t = ThoughtController.new

=> #<'ThoughtController:0x007f95636ae000>

t.thought.id

Thought Load (0.4ms)  SELECT "thoughts".* FROM "thoughts"

=> 7

t.thinker

Thought Load (0.4ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.4ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.3ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"
Thought Load (0.2ms)  SELECT "thoughts".* FROM "thoughts"

=> [5, 9, 4, 7, 4, 5, 8, 7, 7, 1, 6, 1, 8, 6, 4, 1, 2, 7, 3, 2]

And right there in the middle of the array is a repeat. I'm still looking for code to keep from getting the same record twice in a row.

Upvotes: 0

Views: 167

Answers (1)

davidfurber
davidfurber

Reputation: 5518

Since you're loading all of the thoughts, it's not clear why you're returning to the database to fetch your random thought.

def random_thought
  @thoughts ||= Thought.all.shuffle
  @thoughts.pop
end

Note that the @thoughts stores all the thoughts into an instance variable that should still be there next time you call random_thought, within the current request anyway, whereas random_thoughts ||= won't work because it stores the thoughts in a local variable. This means that it disappears every time you ask for a random thought, which causes all the thoughts to be loaded every time, which your log output suggests, thereby increasing the chances that you'll get a repeat, as well as being woefully inefficient. If all the thoughts are stored in @thoughts and shuffled, you're guaranteed never to run until you've popped off all the @thoughts.

Alex D is correct re: efficiency. For the purpose of your experiment it is okay to load all the thoughts; if this were a production site it would be a disaster. For some ideas look at the SQL Antipatterns book, summarized in a slideshare here: http://www.slideshare.net/billkarwin/sql-antipatterns-strike-back

Skip ahead to slide 148. You essentially want to query for the max ID, select an ID based on a random number, and grab the next highest existing record.

Upvotes: 1

Related Questions