1
\$\begingroup\$

I have a Table model that needs a unique code. I have a method creating the code (and making sure it's unique), but I have the same line self.code = rand.to_s[2..5] twice. Is there a neat way to only have that line once?

class Table < ActiveRecord::Base
  before_create :set_code

  private
  def set_code
    self.code = rand.to_s[2..5]
    while(Table.find_by(code: code) != nil)
      self.code = rand.to_s[2..5]
    end
  end
end
\$\endgroup\$
  • \$\begingroup\$ This is a very weird thing to do. Could you tell us more about this table and why you want a random code? \$\endgroup\$ – 200_success Apr 22 '17 at 3:12
  • \$\begingroup\$ It's for charity events where packages are distributed at tables. If there are two devices used for data entry at the same table, I don't want two tables, so the first person will create a table, then the second person can use the unique code to "sign in" to the same table \$\endgroup\$ – Mirror318 Apr 22 '17 at 4:34
  • \$\begingroup\$ @Mirror318 thank you for providing the explanation, would you be able to elaborate a little more on its purpose? Because i don't understand. chrs. why not use the ||= operator which is typically used for this type of situation, if i'm understanding correctly? the code you've written seems very unusual if one doesn't understand clearly what you are trying to do. \$\endgroup\$ – BKSpurgeon Apr 23 '17 at 1:53
2
\$\begingroup\$

It's the same in Ruby as in other languages where you want a loop to always run at least once: Move the condition to the end.

begin
  code = ...
end until(Table.find_by(code: code).nil?)

Ruby's syntax is a little odd ("end until"), but it's the same as a do { ... } while() in C-like languages.

However, the overall system you're using is really strange. If all you want is a unique number for the new table, just use id. That's what it's for. It's set automatically at the database layer, so there'll be no collisions.

If you just want 4 random digits, don't make a random float (which may not have enough digits, e.g. 0.0), convert it to a string and pull out a section. Just call rand(1000..9999).to_s. Or rand(10_000).to_s.rjust(4, '0') to get a number padded with leading zeros if necessary.

However, anything you do in the application layer is vulnerable to race conditions! Between the time you check if a code already exists in the database, and the time you save your record, that code may have been added, giving you duplicates. E.g. 2 threads both check for the code 1234 at the same time, and both see that it's not in the database, so both then save records with that same code. Oops.

The proper way to avoid such things is to add a uniqueness constraint in the database itself, and rescue the resulting RecordNotUnique error that'll get raised.

Here's an article discussing other ways to generate unique tokens, and a follow-up on how to handle collisions.

However: just use id. Or find a gem that generates unique tokens for you.

\$\endgroup\$
  • \$\begingroup\$ I don't want id, because I want a 4 digit number, and 0001, 0002, etc will seem more like ids to users instead of a unique code (aesthetic reasons, i guess?) \$\endgroup\$ – Mirror318 Apr 23 '17 at 11:31
  • \$\begingroup\$ Thanks for the rand(10_000).to_s.rjust(4, '0'). Is it really a race condition? I thought rails would simply wait for the result before saving... I'm puzzled you find this so strange, I thought there are many cases for unique codes e.g. CD-keys, scratch-prize cards, online games to join friend's matches, etc \$\endgroup\$ – Mirror318 Apr 23 '17 at 11:35
  • \$\begingroup\$ One more thing—would it be the same to simply put a single line code until ... ? \$\endgroup\$ – Mirror318 Apr 23 '17 at 11:35
  • \$\begingroup\$ @Mirror318 The other cases you mention use complex systems to generate unique codes - not just random codes that may or may not be unique. Look up UUIDs or linear feedback shift registers for instance. That's a lot more to it than just calling rand-something and naïvely checking the result. And yes it is a race condition. If you're running Rails on a proper server, chances are you're running multiple threads. Each thread waits for its result before saving something, but if multiple do so at the same time, that's a race condition. \$\endgroup\$ – Flambino Apr 23 '17 at 11:47
1
\$\begingroup\$

Why not to just use recursion?

class Table < ActiveRecord::Base
  before_create :set_code

  private
  def set_code
    self.code = rand.to_s[2..5]
    set_code if Table.find_by(code: code)
  end
end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.