Add getSingleScalarResultOrNull method

[arkhamvm]

Handly method for retrieves scalars which may not exist.

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

[arkhamvm]

@derrabus will be merged?

[derrabus]

Sorry that nobody has picked up this PR yet. I'm honestly not nery eager to add more utility methods to our query objects and I can't picture myself using the one proposed here.

Can you elaborate a bit more why you feel we need this method in the ORM?

[arkhamvm]

@derrabus in my case it helps to write more clean client code, my point is to keep try/catch db exceptions at ORM side, so calling function can satisfy single responsibility.

[greg0ire]

We shouldn't use exceptions for flow control, they're expensive.

[arkhamvm]

@greg0ire what do you mean? getOneOrNullResult for examle already use NoResultException. Or you point is to completely remove NoResultException ?

[greg0ire]

Oh right we already do this. But let's not add more code with the same bad practice. I mean, why is no result considered exceptional in the first place?

[greg0ire]

But I also think you can completely remove it. Look at the code of getSingleResult.

        if ($this->_hydrationMode !== self::HYDRATE_SINGLE_SCALAR && ! $result) {
            throw new NoResultException();
        }

So maybe it's just the comment on line 797 that is wrong? Can you find a way to call getSingleScalarResult() and get a NoResultException?

The line was added in 4781dc0

[greg0ire]

After reading the thread at #7871 I am now confused as to what the right way would be. That $hydrationMode vs $this->hydrationMode thing that I didn't spot is hard to grasp.

[greg0ire]

My bad, this behavior is actually proven by tests:

orm/tests/Tests/ORM/Functional/QueryTest.php

Lines 465 to 470 in e7efded

	public function testGetSingleScalarResultThrowsExceptionOnNoResult(): void
	{
	$this->expectException(NoResultException::class);
	$this->_em->createQuery('select a.id from Doctrine\Tests\Models\CMS\CmsArticle a')
	->getSingleScalarResult();
	}

[arkhamvm]

@greg0ire I am not a maintainer of any open source framework, but as far as I understand, their main goal is to save development time by covering the most common use cases. So here are my thoughts:

1. This method really saves time (at least for me). Constant try/catch checks are annoying, in fact it's just dead code.
2. In the projects I've worked on, there are about a hundred getters with such checks in total. For me at least, this is a frequent case (of course, I cannot speak for everyone).
3. I agree that exceptions are expensive (especially in conjunction with xdebug), but I don't see any way to get rid of them quickly. Again based on my experience, cases when business logic sees the difference between the result of null and the result of 0 rows (which will be converted to null) are extremely rare. And they could be checked by a separate method like getRowsCount. If you accept this concept, then you could remove NoResultException completely.

No offense :)

[greg0ire]

I don't see any way to get rid of them quickly.

!?

Wouldn't replacing the body of your method with

        $result = $this->execute(null, $hydrationMode);

        if (! is_array($result)) {
            return $result;
        }

        if (count($result) > 1) {
            throw new NonUniqueResultException();
        }

        return array_shift($result);

be one way to do exactly that?

No offense :)

None taken? Why would I be offended by anything you said?

[arkhamvm]

Wouldn't replacing the body of your method with

Then getSingleScalarResultOrNull will be different from getSingleScalarResult, which in theory could lead to different behavior in the future. If that's ok - I'm ready to rework the PR. Maybe refactor getOneOrNullResult too.

Why would I be offended by anything you said?

Just in case :)

[arkhamvm]

Wouldn't replacing the body of your method with

Unfortunately, based on the getOneOrNullResult method, the execute method can also create a NoResultException:

    public function getOneOrNullResult(string|int|null $hydrationMode = null): mixed
    {
        try {
            $result = $this->execute(null, $hydrationMode);
        } catch (NoResultException) {
            return null;
        }