Code standar issues

kyberman wrote:

Hi Eleo,

• Be careful about basic mistakes that your IDE tells you about:
• throw new Exception without backslash or import means the class doesn't need to be loaded. Use throw new \Exception instead.
• Possibly undefined or never used variables like $field_obj = $index->getField($field_id); are usually grayed inside IDE.
• Be careful about the context where you initialize a variable - doing that before a foreach could result in a weird situation when you need something isolated to a current loop.
• Be careful about when you need to log an error and when to output it for the user to see immediately.
• Also, set the exception message for the user to see, instead of logging only, it's usually more descriptive than e.g. Exception when calling listSchemaFields().
• Try to have your functions as small as possible, rather than the big you have there.
• Inside the create function you set the database connection, so the getDatabaseConnection with direct \Drupal::database() call is not needed, you can rely on $this->connection.
• Instead of $collectionApi->listCollections()->getCollections() and then foreach, just call $collectionApi->getCollection($this->configuration['collection_id']).
• Exception handling is still terrible (e.g. huge try blocks, "submitConfigurationForm" function, or throw an exception inside catch block), please read something about that topic.
• Backend config scheme definition inside YAML is missing (I know, this is not there yet also for Recombee).
• The Recommended branch is 1.0.x - every time you push there, the existing 1.0.x-dev release is updated automatically. There is no reason yet to create new branches like 1.1.x or new 1.1.x-dev release every time you merge a patch (I saw the issue where you created that because of a composer cache problem, but that was not the real issue).
• Unfortunately, it's not possible to delete a branch having a release, so please be careful about doing these operations and stop pushing to 1.0.x and 1.x.
• When you are ready for a non-dev release, it will be named 1.1.0-alpha1 or later the full release 1.1.0.

Additionally:

• "client instance should be created only once (e.g. inside constructor) - move it to the class property"
• nothing changed here, I still can see 4 (x2) instances created every time
• I would create "config" and "client" class properties, to be filled inside the "create" function
• validateConfigurationForm:
• logging is not necessary for the validation

Please review my patch with few minimal changes and ask if something is not clear.