Skip to content

Conversation

@ziaratban
Copy link
Contributor

@ziaratban ziaratban commented Mar 6, 2020

Q A
Is bugfix? no
New feature? yes
Breaks BC? no
Tests pass? yes

example (fetch document from database)

if(!MyActiveRecord::exists(['my condition for pass to where()']),$myRecord)
    return false;
$myRecord->field1 = 'value';

example (no fetch document from database)

if(!MyActiveRecord::exists(['my condition for pass to where()']))
    return false;

this PR is efficient when that PR committed.

@samdark
Copy link
Member

samdark commented Sep 19, 2020

Thanks for contribution but this shortcut doesn't look alright:

  1. It's both find() and exists() while called exists().
  2. There's a pass by reference.
  3. It is not significantly simpler than using find()->where()->exists().
  4. Even if above is OK, that should be implemented in AR overall, not on MongoDB specifically.

Therefore decision is not to accept the pull request.

@samdark samdark closed this Sep 19, 2020
@ziaratban
Copy link
Contributor Author

@samdark thanks.

my goals of writing this method is a simple and efficient programming.
i tested this method on my employee and it had good feedback.

It's both find() and exists() while called exists().

no. it is based on-demand. when you set parameter 2 then one() method called.

There's a pass by reference.

this is main goal. by this feature if you set then you get.

It is not significantly simpler than using find()->where()->exists().

please see this example:

$myUserForDelete = User::find()->where([...])->one();
if(!$myUserForDelete)
    return 'error';
$myUserForDelete->Delete();

or

#low code
#better understanding
if(!User::Exists([...],$myUserForDelete))
     return 'error';
$myUserForDelete->Delete();

Even if above is OK, that should be implemented in AR overall, not on MongoDB specifically.

only php side.

@samdark
Copy link
Member

samdark commented Sep 23, 2020

no. it is based on-demand. when you set parameter 2 then one() method called.

Yes, that's what I mean. It does two things and that is not good.

@ziaratban
Copy link
Contributor Author

thanks for comment.

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