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

Make members of filters and strategies private #51

Merged
merged 3 commits into from
Dec 15, 2021

Conversation

driehle
Copy link
Member

@driehle driehle commented Dec 15, 2021

Caution: BC Break

This makes member os filters and strategies private.

For PropertyName filter

The members properties and exclude are now private. As the class itself is marked final, this doesn't have any consequences. No getter/setter methods are provided.

For AbstractCollectionStrategy

The members which are are now private are:

  • collectionName -> use setCollectionName() / getCollectionName() instead
  • classMetadata -> use setClassMetadata() / getClassMetadata() instead
  • object -> use setObject() / getObject() instead
  • inflector -> use getInflector() instead, or set inflector using the constructor

While AbstractCollectionStrategy is marked as internal in 3.x, it wasn't internal in 2.x, so this will be a BC break for users upgrading from 2.x who have implemented their own strategies extending from AbstractCollectionStrategy.

@driehle driehle added this to the 3.0.0 milestone Dec 15, 2021
@driehle driehle self-assigned this Dec 15, 2021
@@ -43,6 +44,10 @@ public function setCollectionName(string $collectionName): void

public function getCollectionName(): string
{
if ($this->collectionName === null) {
throw new RuntimeException('Collection name has not been set.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From https://matthiasnoback.nl/2018/09/assertions-and-assertion-libraries/

User input will indeed be a reason for functions to fail. But so are external failures in outgoing calls. If a function reaches out to the database to fetch an entity by its ID, then the entity may not exist (anymore) and the call will fail. Before you make that call to the database, you don't know yet if the function will fail or not. This is why language designers usually make a difference between LogicExceptions and RuntimeExceptions. They all extend from the generic Exception class, but their intent is different. A RuntimeException is a failure caused by external, unpredictable things: the database, the filesystem, the network, etc. A LogicException is a programming mistake. It shouldn't have happened, but somehow, somewhere, a programmer didn't use a function well.

Suggested change
throw new RuntimeException('Collection name has not been set.');
throw new LogicException('Collection name has not been set.');

This also applies below.

@driehle driehle merged commit 668eecd into doctrine:3.0.x Dec 15, 2021
@driehle driehle deleted the feature/unit-tests branch December 16, 2021 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants