Problem/Motivation

Constructors become part of an object's public API, but often change as functionality gets added, which in turn generates complexity in the constructor, e.g.: having to declare nullable new parameters at the end, having unused arguments, etc.

Since Elasticsearch Connector major versions (i.e.: release series') follow Elasticsearch's major versions, it may be impossible to introduce breaking changes until Elasticsearch itself releases a new version.

Given that constructors are part of the public API, and we can't introduce breaking changes easily, the potential for complex cruft to build up in them over the lifecycle of an Elasticsearch Connector release series is higher than other modules which don't follow an upstream project's release series as closely.

Fortunately, if we do dependency injection using \Drupal\Core\Plugin\ContainerFactoryPluginInterface::create(), then we can change private/protected properties of an object in the create() function, because the create function is in the same class.

Note, however, that there are a few places where we cannot easily change constructors: events in the Drupal\elasticsearch_connector\Event namespace, and Elasticsearch query-construction objects in the Drupal\elasticsearch_connector\SearchAPI\Query. I propose we leave these more-complex cases out of this issue.

Proposed resolution

Where possible, remove constructors in favor of setting properties in \Drupal\Core\Plugin\ContainerFactoryPluginInterface::create in production and test code (but don't do this in the Drupal\elasticsearch_connector\Event nor Drupal\elasticsearch_connector\SearchAPI\Query namespaces).

Make empty constructors more-compact across the whole project.

Remaining tasks

  1. Merge request 9.0.x - merge request !215 created by @mparker17 in #2
  2. Merge request 8.0.x - merge request !216 created by @mparker17 in #4
  3. Merge to 9.0.x - merged by @mparker17 in #6
  4. Merge to 8.0.x - merged by @mparker17 in #8
  5. Release 9.0.x
  6. Release 8.0.x

User interface changes

None.

API changes

None.

Data model changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mparker17 created an issue. See original summary.

mparker17’s picture

Issue summary: View changes
Status: Active » Needs review

Created merge request !215 for 9.0.x. All the tests pass locally; hopefully in CI as well.

mparker17’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Created merge request !216 for 8.0.x

Tests are passing, so I'm going to merge this.

  • mparker17 committed 6e97597f on 9.0.x
    feat: #3587430 Update constructors and create functions
    
    By: mparker17
    
mparker17’s picture

Version: 9.0.x-dev » 8.0.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Patch (to be ported)

Merging to 8.0.x next.

  • mparker17 committed 93ce5bcd on 8.0.x
    feat: #3587430 Update constructors and create functions
    
    By: mparker17
    
mparker17’s picture

Issue summary: View changes
Status: Patch (to be ported) » Fixed

Merged to 8.0.x. I'll update this issue when this change gets released.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.