Remove usage of "define_method" for defining short, similar methods
Description
# in lib/gitlab/markdown/pipeline.rb
# and lib/banzai/pipeline/base_pipeline.rb
%i(call to_document to_html).each do |meth|
define_method(meth) do |text, context|
context = transform_context(context)
html_pipeline.__send__(meth, text, context) # rubocop:disable GitlabSecurity/PublicSend
end
end
Here we use define_method
instead of explicitly typing three methods with 2 lines each. Every time somebody reads this code they have to pause for a few seconds and think about the reason for using define_method
.
# in app/models/diff_note.rb
%i(original_position position change_position).each do |meth|
define_method "#{meth}=" do |new_position|
if new_position.is_a?(String)
new_position = JSON.parse(new_position) rescue nil
end
if new_position.is_a?(Hash)
new_position = new_position.with_indifferent_access
new_position = Gitlab::Diff::Position.new(new_position)
end
return if new_position == read_attribute(meth)
super(new_position)
end
end
Here we do the same, but it's longer. However, please notice that the larger part of the method does not change based on meth
, it only depends on new_position
. This part can be extracted to a separate private method that is exactly the same for original_position
, position
and change_position
.
Proposal
def call(text, context)
context = transform_context(context)
html_pipeline.call(text, context)
end
def to_document(text, context)
context = transform_context(context)
html_pipeline.to_document(text, context)
end
def to_html(text, context)
context = transform_context(context)
html_pipeline.to_html(text, context)
end
def original_position=(new_position)
new_position = transform_new_position(new_position)
return if new_position == original_position
super(new_position)
end
def position=(new_position)
new_position = transform_new_position(new_position)
return if new_position == position
super(new_position)
end
def change_position=(new_position)
new_position = transform_new_position(new_position)
return if new_position == change_position
super(new_position)
end