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

Removed hacky switch/case, migrated to if/else and early return statements #5860

Merged
merged 5 commits into from
Jun 8, 2016
Merged

Removed hacky switch/case, migrated to if/else and early return statements #5860

merged 5 commits into from
Jun 8, 2016

Conversation

peter-gribanov
Copy link
Contributor

No description provided.

@peter-gribanov
Copy link
Contributor Author

@guilhermeblanco @Ocramius I have no idea why it is done
5e3e8b3#commitcomment-17772924


default:
throw new \InvalidArgumentException("Invalid argument: " . $conn);
if (is_array($conn)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you segregate this entire block into its own private static method?

@peter-gribanov
Copy link
Contributor Author

@Ocramius it's better?

protected static function createConnection($conn, Configuration $config, EventManager $eventManager = null)
{
if (is_array($conn)) {
$conn = DriverManager::getConnection(
Copy link
Member

Choose a reason for hiding this comment

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

return early

@peter-gribanov
Copy link
Contributor Author

@Ocramius now?

return DriverManager::getConnection(
$conn, $config, ($eventManager ?: new EventManager())
);
} elseif ( ! $conn instanceof Connection) {
Copy link
Member

Choose a reason for hiding this comment

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

else not needed, since you return early

}

return new EntityManager($conn, $config, $conn->getEventManager());
return $connection;
Copy link
Member

Choose a reason for hiding this comment

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

Checking whether $conn->getEventManager() === $eventManager is still required (sorry, previous comment may have been misleading)

@Ocramius Ocramius self-assigned this Jun 8, 2016
@Ocramius Ocramius added this to the 2.6.0 milestone Jun 8, 2016
@Ocramius Ocramius changed the title Change switch/case to if/else Removed hacky switch/case, migrated to if/else and early return statements Jun 8, 2016
@Ocramius Ocramius merged commit 7e4106d into doctrine:master Jun 8, 2016
@Ocramius
Copy link
Member

Ocramius commented Jun 8, 2016

@peter-gribanov thanks!

Ocramius added a commit that referenced this pull request Jun 19, 2016
Partially reverting #5860 due to type juggling horrors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants