1
\$\begingroup\$

I'm trying to dry up this Rails post model:

class Post < ActiveRecord::Base
  extend FriendlyId
  friendly_id :title, use: [:slugged, :finders]
  validates_presence_of :title
  scope :published, -> { where('hidden = ? AND published_at IS NOT NULL AND published_at < ?', false, DateTime.now).order(published_at: :desc) }
  paginates_per 10
  attr_accessor :published_at_date, :published_at_time
  acts_as_taggable
  mount_uploader :image, ImageUploader

  after_initialize :set_default_values, :if => :new_record?

  def set_default_values
    self.ad ||= false
  end

  def published?
    if self.hidden == true || self.published_at.blank?
      false
    else
      self.published_at < DateTime.now ? true : false
    end
  end

  def not_published?
    if self.hidden == true || self.published_at.blank?
      true
    else
      self.published_at < DateTime.now ? false : true
    end
  end

  def can_publish?
    !published_at.blank? && hidden?
  end
end

The first thing I am removing is the default value section as that can be changed to a database default to set the initial value of the ad attribute to false each time.

after_initialize :set_default_values, :if => :new_record?

def set_default_values
  self.ad ||= false
end

The next part I am struggling with is the published? and not_published? methods. They should return a boolean value. In trying to try up the published? by removing self and the hidden = true for hidden?. In the else part of the conditional, I remove the ? true : false as the evaluation of the published_at < DateTime.now will return boolean from the method.

def published?
  if hidden? || published_at.blank?
    false
  else
   published_at < DateTime.now
  end
end

The not_published? method follows the same, but flipping the greater than sign to >:

 def not_published?
  if hidden? || published_at.blank?
    true
  else
    published_at > DateTime.now
  end
end

Then I realized that I could just call published? with a not !... duh.

def not_published?
  !published?
end

It is starting to look pretty good but I'd like to reduce it further. How can I reduce the published? method such that it does not need to the false ?

\$\endgroup\$
1
\$\begingroup\$

You could just do:

def published?
  !hidden? && published_at.present? && published_at < DateTime.now
end

However I would probably define something like:

def visible?
  !hidden?
end

or possible even

def visible?
  !hidden? && published_at.present?
end

And then

def published?
  visible? && published_at < DateTime.now
end
\$\endgroup\$
  • \$\begingroup\$ Thanks, I like the visible? as a better option of !hidden. I'll add that as well. \$\endgroup\$ – DogEatDog Oct 2 '17 at 17:45

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.