1
\$\begingroup\$

Hopefully this is not too complicated. I have this code that will generate a username based on the pre-configured schema the admin types in. The fields they can use are:

<first_initial>
<last_initial>
<first_name>
<last_name>
<role>

So, if my name were John Doe, and I had the role of "author" in the system, the function would translate these UsernameSchema thusly:

<first_name>.<last_name>         = john.doe
<first_intitial><last_name>      = jdoe
<first_name>.<last_name>-<role>  = john.doe-author
<last_initial>                   = d

Obviously, it needs to check for duplicates. Below is the working code I have (will be called either after_initialize or before_create (not sure which at this point):

  def set_username
    schema = UsernameSchema.where(:active => true).first! || "<first_initial><last_name>"
    fname = self.first_name
    lname = self.last_name
    finit = fname[0]
    linit = lname[0]
    urole = self.role
    # Question 2:
    replacement_dict = {"<first_initial>" => finit, "<last_initial>" => linit, "<first_name>" => fname, "<last_name>" => lname, "<role>" => urole}
    replacement_regex= /(<first_initial>)|(<last_initial>)|(<first_name>)|(<last_name>)|(<role>)/
    proposed_username = schema.gsub(replacement_regex, replacement_dict).downcase
    self.username = find_free_name(proposed_username)
  end

  # Question 3, IDE complains about next line
  def find_free_name(un)
    user = User.find_by_username(un)
    if user
      num = user.scan(/\d+$/).first
      if num
        new_num = num.to_i + 1
        next_user = un.gsub(num, new_num.to_s)
      else
        next_user = un + "1"
      end
      find_free_name(next_user)
    else
      un
    end
  end

I guess I have these concerns with my code:

  1. Is this a dumb way of doing it?
  2. Where my "Question 2" comment is in the above code, that seems redundant... is there a more succinct way?
  3. My IDE says I'm doing it wrong (and I don't know what it wants):

All custom “finds” accessed from more than one place in the code use named_scope instead of a custom method.

\$\endgroup\$
1
\$\begingroup\$

The code here seems a bit over complicated for its intended purpose.

Would there be some issue with just appending a random number or hash to the end of each user name? something like taking all the user provided information and creating a sha-1 hash then just using the first 3 or so characters?

data = [fname,lname,fint,lint,urole]
Digest::SHA1.base64digest(data.join('')) 
#=> f1ed8d7c48b27af459240984574c7bcd8b14fe4b

This would give you a very unique string to add to each username.

\$\endgroup\$
  • \$\begingroup\$ I never really thought of that. It's not what I'd like to do, but I guess I could do that. I would still need to check for collisions, right? I mean it would be unlikely that I'd have one, but shouldn't I still check? \$\endgroup\$ – Jeff Feb 6 '15 at 15:03

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.