Change visibility of handler classes to protected #1934
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some properties and methods of the handler classes are set to private and some others to protected.
I change the visibility of all of them to protected. I believe this is a better practice, as it allows us to extend the handler.
If there are any security concerns of why this is the way it is, please do let me know.
Example
The
MongoDBHandler
stores the configuration private as shown:monolog/src/Monolog/Handler/MongoDBHandler.php
Lines 37 to 41 in d97d5e9
But the
DynamoDBHandler
stores the configuration protected as shown:monolog/src/Monolog/Handler/DynamoDbHandler.php
Lines 32 to 36 in d97d5e9
With how is the code right now, I can extend the
DynamoDbHandler
to add querying functionality, all without having to create a new configuration object (akaAws\DynamoDb\DynamoDbClient
).But with
MongoDBHandler
, I have to redo the argument parsing in its construct function to include this functionality, creating also two copies of the same exact object in memory (akaMongoDB\Driver\Manager
or\MongoDB\Collection
).