Topic: Help my model

I just want to know if there is anything I can do to DRY up this model, or just make it better in general.

require 'digest/sha1'
class User < ActiveRecord::Base
  belongs_to :state
  belongs_to :ethnicity, :class_name => "Ethnicity", :foreign_key => "ethnicity"
  belongs_to :marital_status, :class_name => "MaritalStatus", :foreign_key => "marital_status_id"
  belongs_to :education, :class_name => "Education", :foreign_key => "education_id"
  belongs_to :income, :class_name => "Income", :foreign_key => "income_id"
  belongs_to :employment, :class_name => "Employment", :foreign_key => "employment_id"
  has_and_belongs_to_many :health_condition, :join_table => "health_conditions_users", :foreign_key => "health_condition_id"
 

  # Virtual attribute for the unencrypted password
  attr_accessor :password
 
 
  validates_presence_of     :login,                       :if => :activated?
  validates_length_of       :login,    :within => 3..40,  :if => :activated?
  validates_presence_of     :email,                       :if => :email_required?
  validates_length_of       :email,    :within => 3..100, :if => :email_required?
  validates_presence_of     :password,                    :if => :password_required?
  validates_presence_of     :password_confirmation,       :if => :password_required?
  validates_length_of       :password, :within => 4..40,  :if => :password_required?
  validates_confirmation_of :password,                    :if => :password_required?
  validates_uniqueness_of   :login, :case_sensitive => false, :if => :login_need_uniqueness?
  validates_uniqueness_of   :email, :case_sensitive => false, :if => :email_need_uniqueness?
  validates_presence_of     :zip,                         :if => :zip_required?
  validates_presence_of     :first_name, :last_name

  validates_presence_of :date_of_birth, :home_phone, :address, :city, :state, :gender, :ethnicity, :if => :completion_required?
  validates_numericality_of :home_phone, :zip, :if => :completion_required?
  validates_length_of :home_phone, :is => 10, :if => :completion_required?
  validates_length_of :zip, :is => 5, :if => :completion_required?
  with_options :if => Proc.new { |User| !User.work_phone.blank? } do |wonu|
    wonu.validates_numericality_of :work_phone, :if => :completion_required?
    wonu.validates_length_of :work_phone, :is => 10, :if => :completion_required?
  end
 
  before_save :encrypt_password
 
 
 
 
  # prevents a user from submitting a crafted form that bypasses activation
  # anything else you want your user to change should be added here.
  attr_accessible :login, :email, :password, :password_confirmation, :zip, :first_name, :last_name, :activated, :date_of_birth, :home_phone, :address, :city, :state_id, :gender, :ethnicity_id, :work_phone, :gender, :education_id, :income_id, :employment_id, :employer, :health_conditions_id, :children, :marital_status_id
 
  # Authenticates a user by their login name and unencrypted password.  Returns the user or nil.
  def self.authenticate(login, password)
    u = find_by_login(login) # need to get the salt
    u && u.authenticated?(password) ? u : nil
  end

  # Encrypts some data with the salt.
  def self.encrypt(password, salt)
    Digest::SHA1.hexdigest("--#{salt}--#{password}--")
  end

  # Encrypts the password with the user salt
  def encrypt(password)
    self.class.encrypt(password, salt)
  end
 
  def completion_required?
    !self.complete_profile && self.activated
  end
 
  def complete
    self.complete_profile = true
    self.save(false)
  end

  def authenticated?(password)
    crypted_password == encrypt(password)
  end

  def activated?
    self.activated
  end
 
  def activate
    self.activated = true
    self.login = self.email
    save(false)
  end
 
  def complete_profile?
    self.complete_profile
  end
 
  def admin?
    self.admin
  end
 
  def db_admin?
    self.db_admin
  end
 
  def added_by_admin?
    self.added_by_admin
  end

  def remember_token?
    remember_token_expires_at && Time.now.utc < remember_token_expires_at
  end

  # These create and unset the fields required for remembering users between browser closes
  def remember_me
    remember_me_for 2.weeks
  end

  def remember_me_for(time)
    remember_me_until time.from_now.utc
  end

  def remember_me_until(time)
    self.remember_token_expires_at = time
    self.remember_token            = encrypt("#{email}--#{remember_token_expires_at}")
    save(false)
  end

  def forget_me
    self.remember_token_expires_at = nil
    self.remember_token            = nil
    save(false)
  end
 
  def create_activation_code
    self.activation_code = rand(99999999)
  end

  protected
    # before filter
    def encrypt_password
      return if password.blank?
      self.salt = Digest::SHA1.hexdigest("--#{Time.now.to_s}--#{login}--") if new_record?
      self.crypted_password = encrypt(password)
    end
   
    def password_required?
       ( crypted_password.blank? && self.activated? ) || ( !password.blank? && self.activated )
     end
     
    def email_need_uniqueness?
      !self.email.blank? && !self.added_by_admin?
    end
   
    def login_need_uniqueness?
      !self.login.blank? && !self.added_by_admin?
    end
   
    def zip_required?
      !self.added_by_admin?
    end
   
    def email_required?
      !self.added_by_admin?
    end
end

"They say, Evil prevails when good men do nothing. What they should have said was: Evil Prevails."

http://www.vrazzle.com

Re: Help my model

Not unless you do some meta-programming, but honestly -- it's fine. Stop fretting, you're not repeating ***** left and right. Don't get too crazy about "dry" smile

http://danielfischer.com - Personal Web-Technology-Blog, Los Angeles.

Re: Help my model

The only part I would clean up are some of your associations at the top, which seem to match rails convention and eliminate the need for you to be so explicit.

belongs_to :marital_status
belongs_to :education
belongs_to :income
belongs_to :employment

Of course if you were a real DRY nazi, you would rather have something like this

belongs_to :marital_status, :education, :income, :employment

But since associations can take blocks as arguments, I'm not sure if this is doable.

Re: Help my model

You could always use with_options to DRY up your validations a little.

  validates_presence_of     :login,                       :if => :activated?
  validates_length_of       :login,    :within => 3..40,  :if => :activated?
  validates_presence_of     :email,                       :if => :email_required?
  validates_length_of       :email,    :within => 3..100, :if => :email_required?

  with_options :if => :password_required? do |pass|
    pass.validates_presence_of :password, :password_confirmation
    pass.validates_length_of :password, :within => 4..40
    pass.validates_confirmation_of :password
  end

  validates_uniqueness_of   :login, :case_sensitive => false, :if => :login_need_uniqueness?
  validates_uniqueness_of   :email, :case_sensitive => false, :if => :email_need_uniqueness?
  validates_presence_of     :zip,                         :if => :zip_required?
  validates_presence_of     :first_name, :last_name

  validates_presence_of :date_of_birth, :home_phone, :address, :city, :state, :gender, :ethnicity, :if => :completion_required?
  validates_numericality_of :home_phone, :zip, :if => :completion_required?
  validates_length_of :home_phone, :is => 10, :if => :completion_required?
  validates_length_of :zip, :is => 5, :if => :completion_required?
  with_options :if => Proc.new { |User| !User.work_phone.blank? } do |wonu|
    wonu.validates_numericality_of :work_phone, :if => :completion_required?
    wonu.validates_length_of :work_phone, :is => 10, :if => :completion_required?
  end


Some metaprogramming might make that even shorter, but it may be overkill in this example.

Last edited by manitoba98 (2007-09-15 17:07:36)

Re: Help my model

Thanks alot! I'm a noob, so its good to know I dont totally suck!

"They say, Evil prevails when good men do nothing. What they should have said was: Evil Prevails."

http://www.vrazzle.com