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

Small correction in DB\Query comments #1598

Merged
merged 1 commit into from
Dec 23, 2013
Merged

Small correction in DB\Query comments #1598

merged 1 commit into from
Dec 23, 2013

Conversation

basil-inc
Copy link
Contributor

I checked this using XDebug, and the function actually returns null (which is something different than false). I assume it is the comment that should be changed, and not the code itself...

I checked this using XDebug, and the function actually returns null (which is something different than false). I assume it is the comment that should be changed, and not the code itself...
qiangxue added a commit that referenced this pull request Dec 23, 2013
Small correction in DB\Query comments
@qiangxue qiangxue merged commit 43ec746 into yiisoft:master Dec 23, 2013
@qiangxue
Copy link
Member

Thanks!

@basil-inc basil-inc deleted the patch-1 branch December 23, 2013 13:11
cebe added a commit that referenced this pull request Dec 26, 2013
@cebe
Copy link
Member

cebe commented Dec 26, 2013

This change is not correct. Reverted and verified in 795c741.

@basil-inc
Copy link
Contributor Author

@cebe this confuses me. Both the docs and Gii-generated code often use the following to check if an AR find has any results:

$employee = Employee::find()->where('id = :id', [':id' => $id])->one();
if ($employee !== null) {
    // do something
}

Since !== also checks the datatype of the variable, this shouldn't work if the query actually returned bool false, right? After all, null is a different datatype than boolean. Please enlighten me why the standard way to check for results is not $result !== false :-)

@cebe
Copy link
Member

cebe commented Dec 26, 2013

AR uses ActiveQuery, not Query. ActiveQuery::one() returns an AR instance or null while Query::one() returns an array or false.

@basil-inc
Copy link
Contributor Author

@cebe alright thanks. Is there any reason why Query doesn't return null too? This would be a logical assumption that beginning Yii developers make (as I just did myself). Most people will learn to work with ActiveRecord first (since the gii-generated code is full of it), so maybe a Query should also return null in the absence of results?

tonydspaniard added a commit to tonydspaniard/yii2 that referenced this pull request Dec 26, 2013
* upstream: (2012 commits)
  doc fix.
  Changed the signature of `urlCreator` and button creators for `yii\gridview\ActionColumn`
  parameter adjustment.
  The signature for `yii\gridview\ActionColumn::urlCreator` is changed - the `$action` parameter is moved to the first
  Fixed the signature of Schema::findUniqueIndexes().
  reverted yiisoft#1598 and added a test for it
  Fix wrong array index in unique indexes for MySql
  Making accesors public
  Get DB unique indexes information
  Fixes yiisoft#1610: `Html::activeCheckboxList()` and `Html::activeRadioList()` will submit an empty string if no checkbox/radio is selected
  Gii should keep horizontal layout
  Documentation at "yii\authclient" updated.
  Doc comments at "yii\authclient" updated.
  Auth clients "Choice" doc comments updated.
  Auth clients "Choice" widget javascript advanced.
  Bootstrap's dropdown encodes also trailing caret tag
  Auth clients "Choice" widget markup updated.
  Gii should keep horizontal layout
  extended from codeception testcase, added docs
  Auth clients for Facebook, GitHub, LinkedIn added.
  ...

Conflicts:
	framework/yii/helpers/BaseInflector.php
	tests/unit/framework/helpers/InflectorTest.php
@cebe
Copy link
Member

cebe commented Dec 28, 2013

Query returns false because it is a wrapper around PDO which returns false in case of no result. AR is another layer over Query and it uses null because it is dealing with objects.

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.

None yet

3 participants