A lot of code in views is using __construct instead of init() even init() is the more recommended way to add things like
an additional_field.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
14.07 KB

I converted all __costructs which are used to add additional fields, but there are some of them which are used for other things like formula etc.
Does anyone has an oppinion whether using __construct for them might be better?

aspilicious’s picture

+++ b/lib/Views/user/Plugin/views/field/Roles.phpundefined
@@ -24,10 +24,10 @@ use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
-  public function __construct(array $configuration, $plugin_id, DiscoveryInterface $discovery) {

You need to remove the "use DiscoveryInterface" in those cases

*My opinion*
Don't load unneeded code. __construct is called once for each object. If init is only called once I don't care if it lives inside the __construct or the init. If the init is (possibly) called multiple times we only should add stuff in there that needs to be rebuild/reset.

dawehner’s picture

Assigned: dawehner » Unassigned
FileSize
18.09 KB

That's a good point!

No init will never be called multiple times or it would be clearly a bug.

tim.plunkett’s picture

I believe there are cases where init() is never actually called, so moving non-essential code to init() is good.

Status: Needs review » Needs work

The last submitted patch, views-1789948-3.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
18.62 KB

Okay tim again broke the code :)

Status: Needs review » Needs work

The last submitted patch, views-1789948-6.patch, failed testing.

tim.plunkett’s picture

Many of the remaining __construct() methods look like they could be converted too, especially the filter code.

dawehner’s picture

Status: Needs work » Needs review
FileSize
18.65 KB

grr

Status: Needs review » Needs work

The last submitted patch, views-1789948-8.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
18.69 KB

One handler had a use missing.

tim.plunkett’s picture

What about

lib/Drupal/views/Plugin/views/argument/ArgumentPluginBase.php
lib/Drupal/views/Plugin/views/argument/Formula.php
lib/Drupal/views/Plugin/views/filter/BooleanOperator.php
lib/Drupal/views/Plugin/views/filter/InOperator.php
lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php
lib/Views/user/Plugin/views/filter/Current.php

?
I guess the Wizard one can wait.

dawehner’s picture

Good question, i just converted the ones with additional_fields but yes it could make sense to convert them as well.

dawehner’s picture

FileSize
24.28 KB

Let's convert them other ones as well.

lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php

Wizards aren't really views plugins, so don't introduce an init method here.

tim.plunkett’s picture

+++ b/lib/Drupal/views/Plugin/views/field/FieldPluginBase.phpundefined
@@ -83,6 +83,7 @@ abstract class FieldPluginBase extends HandlerBase {
   }
 
+
   /**

Extra line here

+++ b/lib/Views/comment/Plugin/views/field/NodeNewComments.phpundefined
@@ -24,11 +24,12 @@ use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
+    $this->additional_fields['comment_count'] = 'comment_count';
     $this->additional_fields['nid'] = 'nid';
     $this->additional_fields['type'] = 'type';
     $this->additional_fields['comment_count'] = array('table' => 'node_comment_statistics', 'field' => 'comment_count');

Isn't this just being written over?

+++ b/lib/Views/translation/Plugin/views/field/NodeTranslationLink.phpundefined
@@ -35,6 +35,7 @@ class NodeTranslationLink extends FieldPluginBase {
   }
 
+
   public function query() {
     $this->ensureMyTable();

Extra line

+++ b/lib/Views/user/Plugin/views/filter/Current.phpundefined
@@ -24,12 +24,13 @@ use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
     $this->value_value = t('Is the logged in user');
+
   }

Extra line here

dawehner’s picture

FileSize
23.68 KB

Regarding the first comment. Actually we just need the last line.

tim.plunkett’s picture

Status: Needs review » Fixed

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