blueprinter icon indicating copy to clipboard operation
blueprinter copied to clipboard

Separate internal options from external ones

Open sandstrom opened this issue 1 year ago • 10 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Is your feature request related to a problem? Please describe

Right now options are used both internally and externally. For example, :block and :association` are blessed options with internal purpose.

They are sometimes 'scrubbed out' for example this line in AssociationExtractor:

options_without_default = options.except(:default, :default_if)

This increases the risk of colliding with user-defined options, and also make the internal code slightly harder to follow/modify.

Describe the feature you'd like to see implemented

I would separate the two sets, in one of two ways:

  1. Either into e.g. internal_options and options, and keep things such as :block in the internal set, or
  2. turn internal options into proper instance variables, such as @field_block (see example below).

I'd prefer the second way, but (1) would at least be an improvement over the current state.

# base.rb
def self.field(method, options = {}, &block)
  current_view << Field.new(
    method,
    options.fetch(:name) { method },
    options.fetch(:extractor) { Blueprinter.configuration.extractor_default.new },
    self,
    inline_definition: block)
  )
end

# field.rb
class Field
  attr_reader :method, :name, :extractor, :options, :blueprint, :inline_definition

  def initialize(method, name, extractor, blueprint, inline_definition: nil, options = {})
    @method = method
    @name = name
    @extractor = extractor
    @blueprint = blueprint
    @options = options
    @inline_definition = inline_definition
  end

Describe alternatives you've considered

No response

Additional context

No response

sandstrom avatar Mar 21 '24 12:03 sandstrom

Happy to discuss this in more detail, and elaborate more.

sandstrom avatar Mar 21 '24 12:03 sandstrom

I'd definitely like to see a bit more intentionality here.options feels like this sort of "grab bag" of information, and I certainly agree that it increases the cognitive overhead when reading through it.

So in that sense, I'd also push more towards your second proposed option.

lessthanjacob avatar Apr 07 '24 20:04 lessthanjacob

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jun 07 '24 01:06 github-actions[bot]

Still relevant!

sandstrom avatar Jun 11 '24 11:06 sandstrom

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Aug 11 '24 01:08 github-actions[bot]

Not stale

sandstrom avatar Aug 14 '24 09:08 sandstrom

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Nov 17 '24 02:11 github-actions[bot]

Not stale

sandstrom avatar Nov 18 '24 09:11 sandstrom

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jan 19 '25 02:01 github-actions[bot]

not stale

sandstrom avatar Jan 19 '25 18:01 sandstrom