Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark eligible classes final #3585

Closed
morozov opened this issue Jun 2, 2019 · 5 comments · Fixed by #3590
Closed

Mark eligible classes final #3585

morozov opened this issue Jun 2, 2019 · 5 comments · Fixed by #3590

Comments

@morozov
Copy link
Member

morozov commented Jun 2, 2019

The issue was originally raised in #3007 (comment).

A quick check with Ocramius/Finalizer shows:

$ finalizer finalizer:check-final-classes lib/
Following classes need to be made final:
+-----------------------------------------------------------------+
| Doctrine\DBAL\Sharding\ShardChoser\MultiTenantShardChoser       |
| Doctrine\DBAL\Sharding\SQLAzure\Schema\MultiTenantVisitor       |
| Doctrine\DBAL\Sharding\SQLAzure\SQLAzureFederationsSynchronizer |
| Doctrine\DBAL\Sharding\PoolingShardManager                      |
| Doctrine\DBAL\Id\TableGeneratorSchemaVisitor                    |
| Doctrine\DBAL\Driver\OCI8\Driver                                |
| Doctrine\DBAL\Driver\Mysqli\Driver                              |
| Doctrine\DBAL\Driver\Mysqli\MysqliStatement                     |
| Doctrine\DBAL\Driver\PDOSqlsrv\Driver                           |
| Doctrine\DBAL\Driver\PDOSqlsrv\Statement                        |
| Doctrine\DBAL\Driver\PDOMySql\Driver                            |
| Doctrine\DBAL\Driver\IBMDB2\DB2Connection                       |
| Doctrine\DBAL\Driver\IBMDB2\DB2Statement                        |
| Doctrine\DBAL\Driver\IBMDB2\DB2Driver                           |
| Doctrine\DBAL\Driver\SQLSrv\SQLSrvStatement                     |
| Doctrine\DBAL\Driver\SQLSrv\Driver                              |
| Doctrine\DBAL\Driver\SQLSrv\SQLSrvConnection                    |
| Doctrine\DBAL\Driver\SQLAnywhere\SQLAnywhereConnection          |
| Doctrine\DBAL\Driver\SQLAnywhere\Driver                         |
| Doctrine\DBAL\Driver\SQLAnywhere\SQLAnywhereStatement           |
| Doctrine\DBAL\Driver\PDOPgSql\Driver                            |
| Doctrine\DBAL\Driver\PDOOracle\Driver                           |
| Doctrine\DBAL\Driver\PDOSqlite\Driver                           |
| Doctrine\DBAL\Driver\StatementIterator                          |
| Doctrine\DBAL\Cache\ResultCacheStatement                        |
| Doctrine\DBAL\Cache\ArrayStatement                              |
| Doctrine\DBAL\Schema\Synchronizer\SingleDatabaseSynchronizer    |
| Doctrine\DBAL\Schema\Visitor\RemoveNamespacedAssets             |
| Doctrine\DBAL\Portability\Statement                             |
+-----------------------------------------------------------------+

For all finalized classes, all their properties and protected methods should be declard private and renamed if they violate the PSR-2 naming conventions.

@mvorisek
Copy link
Contributor

The world is not perfect nor bug free.

Currently, in the atk4/data project, I fixed a lot of issues by extending the specific DB platform. It works well. Most of the issues, I reported here or even proposed a PR.

Such fixing is impossible when the classes are declared final. Would it be possible to remove the final from the driver classes so the oppurtunity to fixing the impl. from the target project side is still possible?

@derrabus
Copy link
Member

I fixed a lot of issues by extending the specific DB platform.

The platform classes are not final, so you can continue to do this.

Would it be possible to remove the final from the driver classes so the oppurtunity to fixing the impl. from the target project side is still possible?

I don't believe that this is really necessary. The DBAL has a well defined driver and middleware architecture that should allow you to easily implement your own driver or hook into whatever the bundled drivers are doing.

If you feel like you need to patch the driver classes, these are your options:

  • Contribute a fix upstream, if there's really something broken.
  • Implement a driver middleware. It should give you the same level of control as overriding public driver methods directly.
  • Fork the driver class you need to patch.

@mvorisek
Copy link
Contributor

One example are driver ExceptionConverter classes. They should not be final.

@derrabus
Copy link
Member

derrabus commented Jul 14, 2022

ExceptionConverter implementations can be decorated. You gain nothing by extending them.

@morozov morozov removed this from the 4.0.0 milestone Aug 12, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants