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

Feature/added shortcuts #11

Merged
merged 1 commit into from
Feb 24, 2020
Merged

Feature/added shortcuts #11

merged 1 commit into from
Feb 24, 2020

Conversation

mmoreram
Copy link
Member

No description provided.

* connection->findOneById('table', ['id' => 1]);
*
* @param string $table
* @param array $id
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd name it $where or $conditions since it doesn't necessarily have to be primary key (same for delete and update) and name the method simply findOneBy.

* @param string $table
* @param array $id
*
* @return array|null
Copy link
Contributor

Choose a reason for hiding this comment

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

@return PromiseInterface<mixed[]|null>

/**
* Find by.
*
* connection->findOneById('table', ['id' => 1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

copy paste error - this is the findBy method

Comment on lines +230 to +220
->values(array_combine(
array_keys($values),
array_fill(0, count($values), '?')
))
->setParameters(array_values($values));
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I prefer my simple foreach over these array shenanigans but whatever.

string $table,
array $id,
array $values
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing return type

* @param array $id
* @param array $values
*
* @return PromiseInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

Promise of...? @return PromiseInterface<...>

@@ -20,7 +20,7 @@
use Exception;

/**
* Class MockedDriver
* Class MockedDriver.
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments like these are totally useless. I highly recommend removing them everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your recommendation. I like these redundant comments 👍

Comment on lines +69 to +71
* @param Connection $connection
*
* @return PromiseInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

@param is redundant since there is typehint, @return should be made non-redundant by adding the type of the resolved value as <...>.

@mmoreram mmoreram force-pushed the feature/added-shortcuts branch from a038338 to 9fe3702 Compare February 24, 2020 17:50
- findOneBy
- findBy
- insert
- update
- upsert
- delete
@mmoreram mmoreram force-pushed the feature/added-shortcuts branch from 9fe3702 to 24c36e9 Compare February 24, 2020 17:55
@mmoreram mmoreram merged commit aed7233 into master Feb 24, 2020
@mmoreram mmoreram deleted the feature/added-shortcuts branch February 24, 2020 17:56
@enumag enumag mentioned this pull request Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants