[DBAL-628] Moved public properties out to getters/setters Created: 11/Oct/13  Updated: 17/Oct/13  Resolved: 17/Oct/13

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: None
Security Level: All

Type: Bug Priority: Trivial
Reporter: Danny Kopping Assignee: Marco Pivetta
Resolution: Won't Fix Votes: 0
Labels: None


 Description   

Having many publicly-accessible properties in most of DBAL's classes makes it difficult to extend the library, and this does not conform to other Symfony projects



 Comments   
Comment by Christophe Coevoet [ 11/Oct/13 ]

Regarding the end of your description, Doctrine DBAL is not a Symfony project. It is a Doctrine project.

Comment by Christophe Coevoet [ 11/Oct/13 ]

And I checked quickly the use of public properties in DBAL 2.4. I only found them in 4 classes: the DebugStack logger and E classes in the Schema tool (ColumnDiff, TableDiff and SchemaDiff), which are value objects (ColumnDiff and TableDiff don't contain any logic at all).
So I would not call it "most DBAL classes". None of the classes which may have a reason to be extended are not using public properties

Comment by Danny Kopping [ 12/Oct/13 ]

Fair enough - I may have been a little hyperbolic in my description.
The problem is that those 4 classes are used throughout the library - and this makes things a little tricky. For example, I wanted to add some functionality where certain tables, columns or indexes could be ignored when generating the migration SQL, and because only public properties were used - one cannot simply override a getter to add an 'ignore' feature.

Any thoughts?

Comment by Christophe Coevoet [ 12/Oct/13 ]

Ignoring some tables is already supported through the schema filter: https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Configuration.php#L92

Comment by Danny Kopping [ 12/Oct/13 ]

Oh awesome - thank you! So you don't see any merit to move all of these out to getters/setters? My particular issue may have been a case of simply overlooking a feature, but could this type of class design not affect things later on? I've already spent a few hours moving the codebase over to getters & setters; if there's no point, i'll quite happily throw away the code.

Comment by Christophe Coevoet [ 12/Oct/13 ]

For value objects, using public properties is fine IMO. They should not hold logic (if you are trying to extend them, you are probably extending the wrong part of DBAL)

Comment by Danny Kopping [ 12/Oct/13 ]

OK, well - I've done the changes and all the tests pass, so I'll push my code up and if at any point these changes are needed - they're be in a PR. Thanks

Comment by Marco Pivetta [ 17/Oct/13 ]

This won't be fixed until 3.x

Generated at Thu Jul 31 15:32:32 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.