Add receiver subsection for Duration Arithmetic

[aeroastro]

We prefer an instance of ActiveSupport::Duration
as a receiver to calculate relative time like 1.minute.after(created_at)

[pirj]

Thank you!

I don't suggest gathering examples of real-world usages, as the "bad" section examples look terrifyingly bad.
Still, if you have time, here is a good source https://github.com/eliotsykes/real-world-rails.

[aeroastro]

Thank you all for the reviews ❤️
I have squashed the commits into 1 commit to be merged.

[andyw8]

I find to hard to say whether either is 'good' or 'bad' without seeing the examples in a wider context.

[aeroastro]

@andyw8

Thank you for the review.
I can show you some example code for this section.

Manage Date of Graduate Programs

https://github.com/gems-uff/sapos/blob/c80f4b586bc1027c6ba19b8d2a6a064ac5cc1b2b/app/models/date_utils.rb#L7-L18

Good Pattern (Original Code)

  def self.add_to_date(date, total_semesters, total_months, total_days)
    if total_semesters != 0
      semester_months = (
        12 * (total_semesters / 2)
      ) + (
        (date.month == 3 ? 5 : 7) * (total_semesters % 2)
      ) - 1
      date = semester_months.months.since(date).end_of_month
    end

    total_days.days.since(total_months.months.since(date).end_of_month)
  end

Diff from Bad to Good

  def self.add_to_date(date, total_semesters, total_months, total_days)
    if total_semesters != 0
      semester_months = (
        12 * (total_semesters / 2)
      ) + (
        (date.month == 3 ? 5 : 7) * (total_semesters % 2)
      ) - 1
-     date = date.since(semester_months.months).end_of_month
+     date = semester_months.months.since(date).end_of_month
    end

-   (date.since(total_months.months).end_of_month).since(total_days.days)
+   total_days.days.since(total_months.months.since(date).end_of_month)
  end

Redmine Milestone Calculation

https://github.com/k41n/redmine_milestones/blob/e5e6eac3de08d8ee0399346bfe58da68bea69690/app/models/milestone.rb#L145-L157

Good Pattern (Original Code)

  def planned_end_date
    if fixed_planned_end_date || planned_end_date_offset.nil?
      read_attribute(:planned_end_date)
    else
      return nil if planned_end_date_offset.nil?
      return nil if previous_planned_end_date_milestone.nil?
      if previous_planned_end_date_milestone.planned_end_date.nil?
        return planned_end_date_offset.days.since(self.previous_planned_end_date_milestone.actual_date) if self.previous_planned_end_date_milestone.actual_date
      else
        return planned_end_date_offset.days.since(self.previous_planned_end_date_milestone.planned_end_date)
      end
    end
  end

Diff from Bad to Good

  def planned_end_date
    if fixed_planned_end_date || planned_end_date_offset.nil?
      read_attribute(:planned_end_date)
    else
      return nil if planned_end_date_offset.nil?
      return nil if previous_planned_end_date_milestone.nil?
      if previous_planned_end_date_milestone.planned_end_date.nil?
-       return self.previous_planned_end_date_milestone.actual_date.since(planned_end_date_offset.days) if self.previous_planned_end_date_milestone.actual_date
+       return planned_end_date_offset.days.since(self.previous_planned_end_date_milestone.actual_date) if self.previous_planned_end_date_milestone.actual_date
      else
-       return self.previous_planned_end_date_milestone.planned_end_date.since(planned_end_date_offset.days)
+       return planned_end_date_offset.days.since(self.previous_planned_end_date_milestone.planned_end_date)
      end
    end
  end

Optional Argument

I have just created an example.

Good Pattern (Original Code)

def calculate_day_after_tomorrow(since: nil)
  if since
    2.days.since(since)
  else
    2.days.since
  end
end

Diff from Bad to Good

def calculate_day_after_tomorrow(since: nil)
  if since
-   since.since(2.days)
+   2.days.since(since)
  else
-   Time.current.since(2.days)
+   2.days.since
  end
end

Expiring Ticket

I have just created an example code

Good Pattern (Original Code)

def purchase_ticket(user, ticket)
  purchased_ticket = PurchasedTicket.new(user_id: user.id, ticket_id: ticket.id, purchased_at: Time.current)
  purchased_ticket.expires_at = ticket.expires_in.seconds.since(purchased_ticket.purchased_at)
  purchased_ticket.save!

  purchased_ticket
end

def extend_expired_at(purchased_ticket, duration)
  purchased_ticket.expires_at = duration.since(purchased_ticket.expires_at)
  purchased_ticket.save!

  purchased_ticket
end

Diff from Bad to Good

def purchase_ticket(user, ticket)
  purchased_ticket = PurchasedTicket.new(user_id: user.id, ticket_id: ticket.id, purchased_at: Time.current)
- purchased_ticket.expires_at = purchased_ticket.purchased_at.since(ticket.expires_in.seconds)
+ purchased_ticket.expires_at = ticket.expires_in.seconds.since(purchased_ticket.purchased_at)
  purchased_ticket.save!

  purchased_ticket
end

def extend_expired_at(purchased_ticket, duration)
- purchased_ticket.expires_at = purchased_ticket.expires_at.since(duration)
+ purchased_ticket.expires_at = duration.since(purchased_ticket.expires_at)
  purchased_ticket.save!

  purchased_ticket
end

[pirj]

For the optional argument example:

def calculate_day_after_tomorrow(since: Time.current)
  2.days.since(since)
end

and 2.days.from_now instead of the no-arg 2.days.since call.

Otherwise, looks good.

Should we put this into a separate guideline?

[aeroastro]

@pirj

Thank you for the review and mentioning the 2.days.from_now.

In my opinion, I would like to focus on 1.minute.since(created_at) for style guide, and also for rubocop-rails

[pirj]

Afair, since is an alias of from_now.
We gave an agreement not to break other guidelines in example code, even in bad if the bad part can be presented differently.