Reviewed code snippets with Best practices

Hello Team,

Last week, I got the chance to review the latest code commits from one of our projects.
This was a helpful step for me to learn good things from it.

Please suggest me, if there are any improvements needed.

Replace instance variable with local variable in partial.

<%= render partial: "review_confirmation", locals: { address: @address } %>

<%= render "review_confirmation", address: @address %>

so that use local variable in partial instead of instance variable.

Best practice to add semicolon ( ; ) at the end of every statement in js file.
Best practice to write css (styles) only in css folder rather than inline css in view file.
Best practice to Use spaces around operators, and curly braces {, }.
Do not use begin blocks where possible.

def send_email_confirmation_of_payment
begin
email_subject = _('subj|payment confirmation for payment # %{reference}')
Mailer.delay.payment_confirmation(email_subject, @user, open_items: @open_items)
flash.now[:info] = _('msg|thanks for payment, email arrives shortly')
rescue Exception => e
logger.error "EMAIL_EXCEPTION;#{eval(Const::App.common_log_part)};
flash.now[:info] = _('msg|thanks for payment')
end
end

we can be improved as:

def send\_email\_confirmation\_of\_payment
   email\_subject = _('subj|payment confirmation for payment # %{reference}')
   Mailer.delay.payment_confirmation(email_subject, @user, open\_items: @open_items)
   flash.now[:info] = _('msg|thanks for payment, email arrives shortly')
 rescue => e
   logger.error "EMAIL\_EXCEPTION;#{eval(Const::App.common_log_part)};
   flash.now[:info] = _('msg|thanks for payment')
 end

Arrange mass-assignments and methods in alphabetical order.

attr\_accessor :credit_limit, :pay_date, :pay_amount, :credit_rep, :credit_cards, :echecks

attr\_accessor :credit_cards, :credit_limit, :credit_rep, :echecks, :pay_amount, :pay_date

Also Methods name should be in alphabetical order.

Should use new syntax of ruby Hash.
Prefer {...} over do...end for single-line blocks. Avoid using {...} for multi-line blocks.
Dont Repeat same code:

def get_availability_mock
@materials.each do |material|
material.in_stock = Quantity.new(1000000)
material.end_lead_time = Date.today + 100
material.min_order_qty = Quantity.new(1000)
material.delivery_unit = Quantity.new(1000)
qty = material.quantity ? material.quantity.value : nil
case qty
when 11000
check_result = AvailabilityCheck::CheckResult.new
check_result.date = Date.today
check_result.quantity = Quantity.new(0)
material.check_results = [ check_result ]
when 12000
check_result = AvailabilityCheck::CheckResult.new
check_result.date = Date.today
check_result.quantity = Quantity.new(2000)
material.check_results = [ check_result ]
else
check_result = AvailabilityCheck::CheckResult.new
check_result.date = Date.today
check_result.quantity = Quantity.new(100000)
material.check_results = [ check_result ]
end
end
end

we can improve as:

def get\_availability\_mock
    @materials.each do |material|
      material.in_stock = Quantity.new(1000000)
      material.end_lead_time = Date.today + 100
      material.min_order_qty = Quantity.new(1000)
      material.delivery_unit = Quantity.new(1000)
      qty = material.quantity ? material.quantity.value : nil
      check\_result = AvailabilityCheck::CheckResult.new
      check\_result.date = Date.today
      case qty
      when 11000
        check\_result.quantity = Quantity.new(0)
      when 12000
        check\_result.quantity = Quantity.new(2000)
      else
        check\_result.quantity = Quantity.new(100000)
      end
      material.check_results = [ check\_result ]
    end
  end

There are two methods which have same code with different names.

 def download
   @open_items = @partner_cache.account_data[@selected_payer_number][@selected_company_code]
    analyze_assigned_payers
    filename = 'Open_items_list'
    headings, columns = open_items_list_headings_and_columns
    download_list(filename, @open_items.open_items, headings, columns)
  end

  def download_pay_invoices_list
    @open_items = @partner_cache.account_data[@selected_payer_number][@selected_company_code]
    analyze_assigned_payers
    filename = 'Open_items_list'
    headings, columns = open_items_list_headings_and_columns
    download_list(filename, @open_items.open_items, headings, columns)
  end

So we can improve the above by using ‘DRY’ (Dont Repeat Yourself) Concepts.

def common\_download\_list
  @open_items = @partner_cache.account_data[@selected_payer_number][@selected_company_code]
  analyze\_assigned\_payers
  filename = 'Open_items_list'
  headings, columns = open\_items\_list\_headings\_and\_columns
  download\_list(filename, @open_items.open_items, headings, columns)
end

def download
  common\_download\_list
end

def download\_pay\_invoices\_list
  common\_download\_list
end