Cms: Inconsistency of return value of one()

[putyourlightson]

Description

According the the docs here, the one() method of an element query should return null if no matching element is found, but in the code the return value is false.

I'm not sure if this is an error in the docs or intended behaviour, but it seems confusing that the one() method of an element query returns false whereas the one() method of an active record query returns null when no matching elements are found. Either way, it seems that either the code or the docs need updating for the case of the former :)

Steps to reproduce

Run the following code:

use craft\elements\Entry;

$entry = Entry::find()
    ->section('news')
    ->slug('no-such-entry')
    ->one();

Craft::dd($entry);

Additional info

• Craft version: 3.0.0-beta.24

[michaelrog]

Sadly, we may be stuck with false, to maintain consistency with Query:one(), which sends back false if the query has no results.

Curiously, though, ActiveQuery:one() returns a null in the event of no results. I'm curious to look into why Yii isn't consistent about this... (ActiveQuery extends Query.)

Either way, I'll take this opportunity to resurrect my campaign for null being a better (more semantic) return value for a query. (Seeing that ActiveQuery extends Query and nonetheless departs from its return values... couldn't ElementQuery also?)

[brandonkelly]

Huh, yeah so technically the docs are incorrect, but I agree null is better, so will keep the docs as-is and change the response to be null, to be consistent with ActiveQuery.

Also, I asked about the inconsistency (which is super rare for Yii) in their Slack and Alex agreed it should be fixed (going with null everywhere). Issue here: yiisoft/yii2#15151

[brandonkelly]

Further evidence null is better – half of the places that called one() were already strictly checking for a null response instead of false. 😞

[brandonkelly]

Now debating whether we should update craft\db\Query::one() to return null as well. Is it better to be consistently incorrect (and keep returning false until Craft 4 / Yii 2.1), or inconsistently correct (and start having it return null)? We’re torn.

[michaelrog]

My vote is to make Craft's API be consistent now (return null everywhere), and let you worry about the internals catching up with Yii whenever 2.1 comes.

(I think it's more likely that a Craft/plugin dev would use Craft's Query and appreciate not having to change their code later, than that a Craft/plugin dev would be using both Craft's _and_ Yii's Query in different places and therefore appreciate parity between the two.)

_?? FOR GREAT VICTORY!_

[brandonkelly]

Of course The Null Guy votes for null

[michaelrog]

I vote for Great Victory.

Shouldn't we all?

[narration-sd]

Hmm...I looked up Great Victory game on Google, and the second item it gave me was Wikipedia on Pyrrhic Victory; the fourth was Elusive Victory....! n.b. among other things, I guess it's not a game...

Not just to chime in, this move sounds very appropriate. Null indeed has a important meaning, in SQL land, anyway -- that data isn't present, not that it's zero or logical false.

I'm confident this was invented to prevent accountants from using incomplete data wrongly, resulting in misleading reports, but anyway it's something you have to understand to get expected results from nested subqueries, as an example.

Yii's query builder does those well if a bit obscurely, so great to keep the truth value in there along with the possible data value -- cheers and a smile for going for it, @brandonkelly

Side benefit: accountants of the world are now safe with Craft (some of my best friends....obligatory) :)

[putyourlightson]

Thanks for changing the code Brandon. Regarding craft\db\Query::one(), my vote is to maintain consistency with Yii (correctness seems subjective in this case). As much as I would like to see it changed to null, I understand cebe's argument, so I would see what they decide and follow suit with the framework.

[narration-sd]

yes, and @cebe has this right with a good operating example via the null coalescing operator, while @creocoder has the accurate flavor underlying in the original here