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);
	}
}

$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 :)