2
\$\begingroup\$

I was building a rails web, and used form object to separate the validation logics per form.

I had to find_referral_code twice in order to store the user_id of referrer. I wonder how I can improve it.

after_sign_up_form.rb

def get_referred_by_id
    User.where(referral_code: referer_referral_code).first.id
end

def validates_presence_of_referer_referral_code
    u = User.find_by_referral_code(referer_referral_code)
    if u.nil?
        errors.add :referer_referral_code, "Not a valid Referral Code"
    else 
        if @user.referral_code == referer_referral_code
            errors.add :referer_referral_code, "Can't use your own referral code"
        end
    end
end

def validates_unique_primary_email
    # errors.add :primary_email, "Email is taken"
end

def email_is_self
    (@user.identities.pluck(:email) << @user.email).flatten.include? primary_email
end
\$\endgroup\$
3
\$\begingroup\$

You can memorize result using ||=

def referred_by_id
    @reffered ||= User.where(referral_code: referer_referral_code).first.id
end

def validates_presence_of_referer_referral_code
    u = referred_by_id
    if u.nil?
        errors.add :referer_referral_code, "Not a valid Referral Code"
    else 
        if @user.referral_code == referer_referral_code
            errors.add :referer_referral_code, "Can't use your own referral code"
        end
    end
end

You shouldn't name your method with get_ prefix.


EDITED:

  • It is ruby naming convention to not use prefixes like get_ nor set_ (according to Rubocop and Ruby style guide). I think it is because, every method is a getter by default (always returns last line).

  • When you call referred_by_id method, it will query a db just once and then it will memorize result. So it will improve performance if this method is called more then once in this class.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ would you please explain why you shouldn't prefix (edit it in your answer please)? and also what benefits come from Memorizing the result using ||= ? \$\endgroup\$ – Malachi Jun 12 '15 at 15:01
  • 1
    \$\begingroup\$ @Lyle'sMug I've edited my answer \$\endgroup\$ – caspg Jun 12 '15 at 15:23
  • \$\begingroup\$ A better explanation for the convention of not including get_… in method names is that a setter, if it existed, would be def referred_by_id=. \$\endgroup\$ – 200_success Jun 12 '15 at 15:26
  • \$\begingroup\$ also, in ruby there is no notion of "properties" that differ from instance methods, as in Java or Javascript for instance. The set/get convention is a workaround to avoid confusing properties with methods in the first place ("do I have to use braces after this.title ? Is it a method or a property ?"). Ruby ditched this notion of property in favor of instance variables that can only be accessed through instance methods, following the principle of least surprise and smalltalk's "message passing" paradigm. \$\endgroup\$ – m_x Jul 7 '15 at 13:48

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.