Suggestion for how to get rid of magic __call functions in TYPO3 Flow repository classes

Repository.php

<?php
class Repository {
	public function __call($method, $arguments) {
		$query = $this->createQuery();
		$caseSensitive = isset($arguments[1]) ? (boolean)$arguments[1] : TRUE;

		if (substr($method, 0, 6) === 'findBy' && strlen($method) > 7) {
			$propertyName = lcfirst(substr($method, 6));
			return $query->matching($query->equals($propertyName, $arguments[0], $caseSensitive))->execute();
		} elseif (substr($method, 0, 7) === 'countBy' && strlen($method) > 8) {
			$propertyName = lcfirst(substr($method, 7));
			return $query->matching($query->equals($propertyName, $arguments[0], $caseSensitive))->count();
		} elseif (substr($method, 0, 9) === 'findOneBy' && strlen($method) > 10) {
			$propertyName = lcfirst(substr($method, 9));
			return $query->matching($query->equals($propertyName, $arguments[0], $caseSensitive))->execute()->getFirst();
		}

		trigger_error('Call to undefined method ' . get_class($this) . '::' . $method, E_USER_ERROR);
	}
}

RepositoryNew.php

<?php
class Repository {
 
	/**
	 * @param string $propertyName
	 * @param boolean $caseSensitive
	 * @return QueryResultInterface
	 * @api
	 */
	public function findBy($propertyName, $caseSensitive = TRUE) {
		$query = $this->createQuery();
		return $query->matching($query->equals($propertyName, $propertyName, $caseSensitive))->execute();
	}
 
	/**
	 * @param string $propertyName
	 * @param boolean $caseSensitive
	 * @return \object
	 * @api
	 */
	public function findOneBy($propertyName, $caseSensitive = TRUE) {
		$query = $this->createQuery();
		return $query->matching($query->equals($propertyName, $propertyName, $caseSensitive))->execute()->getFirst();
	}
 
	/**
	 * @param string $propertyName
	 * @param boolean $caseSensitive
	 * @return integer
	 * @api
	 */
	public function countBy($propertyName, $caseSensitive = TRUE) {
		$query = $this->createQuery();
		return $query->matching($query->equals($propertyName, $propertyName, $caseSensitive))->count();
	}
	
	/**
	 * @param string $method Name of the method
	 * @param array $arguments The arguments
	 * @return mixed The result of the repository method
	 * @deprecated blah blah
	 */
	public function __call($method, $arguments) {
		$caseSensitive = isset($arguments[1]) ? (boolean)$arguments[1] : TRUE;
 
		if (substr($method, 0, 6) === 'findBy' && strlen($method) > 7) {
			$propertyName = lcfirst(substr($method, 6));
			return $this->findBy($propertyName, $caseSensitive);
		} elseif (substr($method, 0, 7) === 'countBy' && strlen($method) > 8) {
			$propertyName = lcfirst(substr($method, 7));
			return $this->countBy($propertyName, $caseSensitive);
		} elseif (substr($method, 0, 9) === 'findOneBy' && strlen($method) > 10) {
			$propertyName = lcfirst(substr($method, 9));
			return $this->findOneBy($propertyName, $caseSensitive);
		}
 
		trigger_error('Call to undefined method ' . get_class($this) . '::' . $method, E_USER_ERROR);
	}
}

Undeclared variable $arguments is not transferred to new public methods; result inoperable. You need to rethink this pattern. I also disagree about making $caseSensitive an argument on API. If we're going to remove magic call methods, we should use the opportunity to also get rid of magic function arguments. Custom searches should require ->createQuery() and manipulating this - or, as always preferred, actual spelled-out methods on each Repository.

I just typed this down here as gist. This obviously needs to be covered by tests and documented - I fixed the gist anyways, thanks!
$caseSensitive is not a magic argument at all and it's important to be able to explicitly make case insensitive lookups IMO. But I agree, custom findBy* methods should be explicitly implemented in order to get real IDE support (e.g. no "@return \object"). This should also be communicated as best practice.

$caseSensitive is not a magic argument at all and it's important to be able to explicitly make case insensitive lookups IMO.

TBH this is an assumption, but: does case insensitive "findBy" not only apply to ValueObject and if so, does it truly apply to Repository? Might it be better to make case insensitive lookups the default behaviour when relation is a ValueObject and otherwise make official practice "use createQuery or make custom method on Repository"?

But I agree, custom findBy* methods should be explicitly implemented in order to get real IDE support (e.g. no "@return \object"). This should also be communicated as best practice.

Ab-so-freakin'-lutely. It is just better in every way: tests, IDE support, documentation rendering - everything.

I'm not perfectly sure about the caseSensitive flag to be honest. But it feels quite like a very common use case to me and I think that case sensitivity can be part of the domain logic (e.g. if you have a repository of articles, wikipedia-style, case sensitivity is really important). I also doubt whether TRUE is a good default here, but I'm sure Karsten has thought about this

FYI: a little benchmark comparing magic methods to actual implementations: https://gist.github.com/bwaidelich/7334680

No doubt: magic methods are extremely slow (and all the other things that ails them). More and more I lean towards making actual methods a requirement instead - and simply dropping these magic methods in a future version. Thanks for the benchmarking, it's nice to see just how bad it can get in practice :)