Fix behaviour of `cycle` function & mappings ?

[smnandre]

I have a couple questions about the "cycle" function...

I'm not sure what is really expected in some scenarios (mainly "what to do with mappings"), and the documentation uses "mapping" "sequence" and "list" terms.

I'll gladly open a PR, but i'm not sure what to do here :)

---

Current code of the cycle function here

```php /** * Cycles over a value. * * @param \ArrayAccess|array $values * @param int $position The cycle position * * @return string The next value in the cycle * * @internal */ public static function cycle($values, $position): string { if (!\is_array($values) && !$values instanceof \ArrayAccess) { return $values; } if (!\count($values)) { throw new RuntimeError('The "cycle" function does not work on empty sequences/mappings.'); } return $values[$position % \count($values)]; } ```

---

ArrayAccess ?

If $values implements ArrayAccess but not Countable, depending on the PHP version the effects can be a simple warning or ... a DivisionByZeroError

• Genuine cursiosity: what is the use case with ArrayAccess here ?
• Should we check is_countable first ?
• Or should this be added in the docblock ?
• Should we deprecate passing a not countable ArrayAccess ?

Sequences / Mappings

There is nothing explicit in the docblock, and the exception is about "sequences/mappings" but .. in reality this method does only works with lists (sequences) of strings.

• Should this be checked before accessing the result / counting items ?
• Or inversely, if $values is a mapping, should we convert it to a sequence first ?
• Should the "position" value be tested ? (a negative value will not work here)
• Should the 'string' return type be enforced / checked somewhere ?

[stof]

I would suggest removing the return type, as there is no reason to restrict it to strings (we could want to cycle over a list of booleans).

I would suggest documenting that this function expects a sequence and a non-negative position (not sure whether we need runtime checks for those, I will let @fabpot decide).

We should indeed forbid passing a non-countable ArrayAccess (with a deprecation first).

If $values implements ArrayAccess but not Countable, depending on the PHP version the effects can be a simple warning or ... a DivisionByZeroError

you cannot get a DivisionByZeroError as computing the count to 0 will throw a RuntimeError before reaching the modulo.

[fabpot]

I would suggest removing the return type, as there is no reason to restrict it to strings (we could want to cycle over a list of booleans).

Agreed

I would suggest documenting that this function expects a sequence and a non-negative position (not sure whether we need runtime checks for those, I will let @fabpot decide).

Agreed to document that (without runtime checks)

We should indeed forbid passing a non-countable ArrayAccess (with a deprecation first).

Agreed

[fabpot]

@smnandre Do you want to work on a PR?

[smnandre]

Yep :)