search

doctrine / dbal

Doctrine Database Abstraction Layer
https://www.doctrine-project.org/projects/dbal.html
MIT License
9.44k stars 1.33k forks source link

The Driver::connect() method signature is redundant #4069

Closed morozov closed 4 years ago

morozov commented 4 years ago
Q A
BC Break no
Version 2.10.2

User credentials and driver options are passed to the driver both via individual parameters and as part of $params: https://github.com/doctrine/dbal/blob/48625f1bc761f1f8f70fe6630a0f3a3204d9a6ff/src/Connection.php#L352-L356

On the one hand, it's unclear to the implementors of Driver::connect(), whether the parameters should be taken from $params or from the individual parameters. On the other hand, the code components that call connect() need to mimic the behavior of the Connection class:

  1. https://github.com/doctrine/dbal/blob/d6f232e4066b2d6ee6d75371c03657d3dd8db7cc/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php#L195-L198
  2. https://github.com/doctrine/dbal/blob/3d8c57560d46f9cdd299e6e26de49f2ec25b349f/tests/Functional/Driver/AbstractDriverTest.php#L34-L37
More examples: 3. https://github.com/doctrine/dbal/blob/48625f1bc761f1f8f70fe6630a0f3a3204d9a6ff/tests/Functional/Schema/SchemaManagerFunctionalTestCase.php#L120-L123 4. https://github.com/doctrine/dbal/blob/48625f1bc761f1f8f70fe6630a0f3a3204d9a6ff/tests/Functional/Schema/SqliteSchemaManagerTest.php#L53-L56 5. https://github.com/doctrine/dbal/blob/f88dc282080826528fe7326cb9a7c465aa3074d7/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php#L63-L68 6. https://github.com/doctrine/dbal/blob/f88dc282080826528fe7326cb9a7c465aa3074d7/tests/Doctrine/Tests/DBAL/Driver/PDOPgSql/DriverTest.php#L96-L101 7. https://github.com/doctrine/dbal/blob/f88dc282080826528fe7326cb9a7c465aa3074d7/tests/Doctrine/Tests/DBAL/Functional/Driver/PDOSqlsrv/DriverTest.php#L47-L52

By being this verbose, this API doesn't solve any design problem. On the contrary, once a driver receives these parameters individually, it may have to pack them back or regroup otherwise before processing:

  1. https://github.com/doctrine/dbal/blob/747b0420188e8775c71c0190abb62333eda52cf6/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Driver.php#L17-L19
  2. https://github.com/doctrine/dbal/blob/4074ae933de3bef5130bf99a20c797f6b8a4adc7/lib/Doctrine/DBAL/Driver/SQLSrv/Driver.php#L34-L40
  3. https://github.com/doctrine/dbal/blob/4074ae933de3bef5130bf99a20c797f6b8a4adc7/lib/Doctrine/DBAL/Driver/SQLAnywhere/Driver.php#L78-L84

Since Driver::connect() plays the role of an adapter between the DBAL Connection::connect() and the Driver Connection::__construct(), it shouldn't expect Connection::connect() to do any adaptation.

github-actions[bot] commented 2 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.