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

Add update & delete to Store class #78

Closed
Timu57 opened this issue Jan 12, 2021 · 5 comments
Closed

Add update & delete to Store class #78

Timu57 opened this issue Jan 12, 2021 · 5 comments

Comments

@Timu57
Copy link
Member

Timu57 commented Jan 12, 2021

For simple queries conveniently update and delete documents without the need of QueryBuilder.

Update one or multiple documents.

function update($documents): bool

Delete one document.

function deleteDocument($id, $returnOption = Query::DELETE_RETURN_RESULTS): array|bool|int

For more return options look into #77

@Timu57 Timu57 added the feature label Jan 12, 2021
@Timu57 Timu57 added this to the Version 2.0 milestone Jan 12, 2021
@Timu57 Timu57 added this to To do in Version 2.0 via automation Jan 12, 2021
@Timu57 Timu57 self-assigned this Jan 12, 2021
@rakibtg
Copy link
Member

rakibtg commented Jan 12, 2021

@Timu57 deleteDocument would it be a public API or internal method?

@Timu57
Copy link
Member Author

Timu57 commented Jan 12, 2021

@rakibtg it will be a public method.
Because the Store class already has a delete method to delete a store we can not name the method delete and have to name it differently.

I also thought about naming the method deleteById

What do you think what could be the best fitting name?

@rakibtg
Copy link
Member

rakibtg commented Jan 12, 2021

@Timu57 Can't we already do that? Example:

$qb = $store->getQueryBuilder();

$foo = $qb->where("name", "=", "Foo")->getQuery();

print_r($foo->fetch());

$foo->delete();

@Timu57
Copy link
Member Author

Timu57 commented Jan 12, 2021

@rakibtg yes we can.
But the following is much more convenient:

$store->deleteById($id);

We also could create a deleteBy method:

function deleteBy($criteria, $returnOption = Query::DELETE_RETURN_BOOL): bool|int|array

@rakibtg
Copy link
Member

rakibtg commented Jan 12, 2021

@Timu57 I have mixed feeling for findBy or deleteBy because

  • it might be a query consistency issue
  • What if we add more conditional methods, or if i want to add search in the findBy method then every time we will need to add a new parameter, which i think would create more confusion

@Timu57 Timu57 moved this from To do to In progress in Version 2.0 Jan 13, 2021
@Timu57 Timu57 moved this from In progress to Review in Version 2.0 Jan 13, 2021
@Timu57 Timu57 moved this from Review to Done in Version 2.0 Jan 13, 2021
@Timu57 Timu57 closed this as completed Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Version 2.0
  
Done
Development

No branches or pull requests

2 participants