Code
#
Class StructureRequires should be at the top of the file, module inclusions right immediately after the class is opened, followed by relationships, validations, scopes, and public instance methods. Protected methods should be declared after all public methods, followed by private methods.
Helper methods should be declared in executation-order so that a class can be read from top-to-bottom. Secondary helper methods that are only referred to by private helper methods should be at the bottom of the class.
This ensures that every class in the codebase has the same structure and there are no surprises.
require '...' # if any
# class documentationclass Class include ... # if any
# relationships
# validations
# scopes
# public method documentation def public_method private_method1 private_method2 end
protected
def protected_method ... end
private
def private_method1 ... end
def private_method2 ... endend
#
Human ReadableCode should be strive to be written as similar to standard prose as possible.
Good
Example
def issue_to(user) @user = user user.redis_lock do return nil unless issuable? create_issuance create_reward send_email send_notification endend
Bad
Example
Highlights a few things not to do:
- undescriptive variable names
- high ABC for a single method. methods should be slim and descriptive
- all logic is embedded into public interface - leverage helpers
def issue_to(verbose_variable_name_for_user, _options={}) x = verbose_variable_name_for_user Redis::Lock.new( "User-#{x.id}", expiration: 5.minutes, timeout: 30.seconds ).lock do if program.active? && target.present? && target.users.where(id: x.id).exists? && !program.program_users.find_by(user: x)&.issuances&.exists? && (program.redeem&.end_at.blank? || program.redeem.end_at.future?) issuance = program_user.issuances.create(issued_at: Time.current) if program.redeem.present? && !program.rewards.find_by(user: x) state = Reward::REWARD_STATE[ program.redeem.redeemable? ? :delivered : :unavailable ] @reward = program.rewards.create( user: x, merchant: m, progress: 100, delivered_at: Time.current, state: state, retire_after: ( retire_after_days&.days&.from_now, ) program_issuance: issuance ) Target::CreateExclusion.call(merchant: m, user: x) end if ( program.notify_email.present? && x.email.present? && program_user.issuances.joins(:email).where( emails: { recipient: x } ).none? ) c = Email::Template::GenericCampaign if program.notify_email.configuration.present? c = Email::Html::Campaign end c.create(recipient: x, source: issuance) end if @reward.present? UserNotifier.call( source: issuance, action: :adhoc_reward_issued, user: x ) end end endend