-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor section definitions (extract sections) #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,41 +3,35 @@ | |
class Tram::Page | ||
extend ::Dry::Initializer | ||
|
||
require_relative "page/section" | ||
|
||
class << self | ||
attr_accessor :i18n_scope | ||
|
||
def section(name, options = {}) | ||
@__sections ||= [] | ||
|
||
n = name.to_sym | ||
if @__sections.map(&:first).include?(n) | ||
raise "Section #{n} already exists" | ||
end | ||
name = name.to_sym | ||
raise "Section #{name} already exists" if sections.key? name | ||
|
||
n = define_section_method(n, options) | ||
@__sections << [n, options] | ||
section = Section.new(name, options) | ||
sections[name] = section | ||
define_method(name, §ion.block) if section.block | ||
end | ||
|
||
def url_helper(name) | ||
raise "Rails url_helpers module is not defined" unless defined?(Rails) | ||
delegate name, to: :"Rails.application.routes.url_helpers" | ||
end | ||
|
||
private | ||
|
||
def define_section_method(n, options) | ||
return n unless options[:value] | ||
define_method(n) { instance_exec(&options[:value]) } | ||
n | ||
def sections | ||
@sections ||= {} | ||
end | ||
end | ||
|
||
def to_h(options = {}) | ||
data = page_methods(options).map do |(name, opts)| | ||
value = public_send(opts[:method] || name) | ||
[name, value] | ||
end | ||
Hash[data] | ||
def to_h(except: nil, only: nil, **) | ||
obj = self.class.sections.values.map { |s| s.call(self) }.reduce({}, :merge) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, it is worth to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
obj = obj.reject { |k, _| Array(except).map(&:to_sym).include? k } if except | ||
obj = obj.select { |k, _| Array(only).map(&:to_sym).include? k } if only | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
obj | ||
end | ||
|
||
private | ||
|
@@ -47,21 +41,5 @@ def t(key) | |
I18n.t key, scope: [Tram::Page.i18n_scope, self.class.name.underscore] | ||
end | ||
|
||
def page_methods(options) | ||
methods = self.class.instance_variable_get(:"@__sections") || [] | ||
except = Array(options[:except]) | ||
only = Array(options[:only]) | ||
methods.reject do |(name, opts)| | ||
(except.any? && except.include?(name)) || | ||
(only.any? && !only.include?(name)) || | ||
__hide?(opts) | ||
end | ||
end | ||
|
||
def __hide?(opts) | ||
black, white = opts.values_at(:unless, :if) | ||
(black && public_send(black)) || (white && !public_send(white)) | ||
end | ||
self.i18n_scope = "pages" | ||
end | ||
|
||
Tram::Page.i18n_scope = "pages" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
# frozen_string_literal: true | ||
|
||
class Tram::Page | ||
# | ||
# Contains class-level definition (name and options) for a section | ||
# with a method [#call] that extracts the section part of hash | ||
# from an instance of [Tram::Page] | ||
# | ||
class Section | ||
extend Dry::Initializer | ||
param :name, proc(&:to_sym) | ||
option :method, proc(&:to_s), default: -> { name }, as: :method_name | ||
option :value, proc(&:to_proc), optional: true, as: :block | ||
option :if, proc(&:to_s), optional: true, as: :positive | ||
option :unless, proc(&:to_s), optional: true, as: :negative | ||
option :skip, true.method(:&), optional: true | ||
|
||
# @param [Tram::Page] page | ||
# @return [Hash] a part of the section | ||
def call(page) | ||
skip_on?(page) ? {} : { name => value_at(page) } | ||
end | ||
|
||
private | ||
|
||
def skip_on?(page) | ||
return true if skip | ||
return true if positive && !page.public_send(positive) | ||
return true if negative && page.public_send(negative) | ||
end | ||
|
||
def value_at(page) | ||
block ? page.instance_exec(&block) : page.public_send(method_name) | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is somewhat not optimal to calculate all values and then throw out some or most of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh! Yes, you are right
It would be better to filter out sections, not calculated values