fix warning about $tmp in each-helper

[d1rk]

$tmp might be an object that can be iterated, but array_keys expects to receive an array, so we cast it like that to suppress warnings.

[everplays]

The thing that I don't like about casting to an array is memory issue. For example imagine that the Traversable object is an interface to iterate over a database query with many rows or a big file, doing this will store everything in memory which is opposite to what Traversables are designed for.

I'm not sure if there's another way to fix it (and I think the casting thing happens right now, anyway), so if you think that it's the only way too, I'll merge it.

[JustBlackBird]

I think that all objects should be treated as lists. This is how Handlebars.js works. Thus the checking, updated in the PR, should looks like:

$islist = ($tmp instanceof \Generator) || (is_array($tmp) && (array_keys($tmp) == range(0, count($tmp) - 1)));

This approach solves the problem with warnings without potential memory issues and make Handlebars.php works more like Handlebars.js

[everplays]

Although Iterator::key can return scalar types (which string is one too) and in that case we should have $islist === false but I agree with @JustBlackBird that, in general, Traversable objects should be treated as lists. So we only need to change the $tmp instanceof \Generator to $tmp instanceof \Traversable.

[JustBlackBird]

@everplays, correct me if i am wrong but as i see $islist === true indicates that container contains only sequential numeric keys. So one cannot replace $tmp instanceof \Generator to $tmp instanceof \Traversable because it will have opposite result. In this case all objects will treated as an array with only sequential numeric keys.

[everplays]

You're right but we've to choose between treating Traversable objects either as numeric indexed arrays or assoc arrays. In my opinion it's more likely that numeric ones get used. For example, a query or file iterator will use numbers as keys not strings. On Feb 23, 2014 1:59 AM, "Dmitriy S. Simushev" notifications@github.com wrote:

@everplays https://github.com/everplays, correct me if i am wrong but as i see $islist === true indicates that container contains only sequential numeric keys. So one cannot replace $tmp instanceof \Generator to $tmp instanceof \Traversable because it will have opposite result. In this case all objects will treated as an array with only sequential numeric keys.

— Reply to this email directly or view it on GitHubhttps://github.com/XaminProject/handlebars.php/pull/50#issuecomment-35817120 .

[everplays]

Also don't forget that "Generator extends Iterator extends Traversable", so we're already treading some Traversables as $islist === true. On Feb 23, 2014 2:06 AM, "Behrooz" everplays@gmail.com wrote:

You're right but we've to choose between treating Traversable objects either as numeric indexed arrays or assoc arrays. In my opinion it's more likely that numeric ones get used. For example, a query or file iterator will use numbers as keys not strings. On Feb 23, 2014 1:59 AM, "Dmitriy S. Simushev" notifications@github.com wrote:

@everplays https://github.com/everplays, correct me if i am wrong but as i see $islist === true indicates that container contains only sequential numeric keys. So one cannot replace $tmp instanceof \Generatorto $tmp instanceof \Traversable because it will have opposite result. In this case all objects will treated as an array with only sequential numeric keys.

— Reply to this email directly or view it on GitHubhttps://github.com/XaminProject/handlebars.php/pull/50#issuecomment-35817120 .

[JustBlackBird]

I think that all \Traversable objects should be treated as associative arrays.

There is one more thing we should take into account. Handlebars.js provide "@index" key for all containers (both objects and arrays). Here is a proof fiddle: http://jsfiddle.net/5e7vS/1/ . I think i can fix this difference between PHP and JS versions soon.

Considering this fact we can provide "@index" for all containers and additionally provide "@key" for \Traversable and arrays with non-sequential or non-numeric keys.

[everplays]

How can you detect that a Traversable is providing integers or other scalar types for keys? On Feb 23, 2014 2:08 PM, "Dmitriy S. Simushev" notifications@github.com wrote:

I think that all \Traversable objects should be treated as associative arrays.

There is one more thing we should take into account. Handlebars.js provide "@index https://github.com/index" key for all containers (both objects and arrays). Here is a proof fiddle: http://jsfiddle.net/5e7vS/1/ . I think i can fix this difference between PHP and JS versions soon.

Considering this fact we can provide "@index https://github.com/index" for all containers and additionally provide "@key https://github.com/key" for \Traversable and arrays with non-sequential or non-numeric keys.

— Reply to this email directly or view it on GitHubhttps://github.com/XaminProject/handlebars.php/pull/50#issuecomment-35828613 .

[JustBlackBird]

I don't think we need to determine type of keys for \Traversable. We can introduce new internal counter for elements and use its values as "@index". Keys of \Traversable will be used as "@key" no matter what type they have. This behavior is more like Handlebars.js.

[everplays]

So you're saying that we set @key even if index of Traversable is integer? I don't think that it be a good idea, there are Iterators that if you cast them to array, would give you an array with numeric indexes. here's an example:

$x = new ArrayIterator(array('egg', 'milk', 'flour'));
var_dump($x instanceof Traversable); // true
var_dump((array) $x); // numerical indexed array

just like jsfiddle example that you provided, I think in such cases the @key should be null.

On Feb 23, 2014 2:29 PM, "Dmitriy S. Simushev" notifications@github.com wrote:

I don't think we need to determine type of keys. We can introduce new internal counter for elements and use its values as "@indexhttps://github.com/index". Keys of \Traversable will be used as "@key https://github.com/key" no matter what type they have. This behavior is more like Handlebars.js.

— Reply to this email directly or view it on GitHubhttps://github.com/XaminProject/handlebars.php/pull/50#issuecomment-35828995 .

[JustBlackBird]

In Handlebars.js even if an object has only integer keys, "@key" will be provided for it. Here is updated fiddle http://jsfiddle.net/75qha/1/

I think that if we provide for \Traversable instances more info ("@index" + "@key") it will be better than if we provide less info (only "@index"). Only "@index" can be provided for arrays with sequential numeric keys. I believe it's Handlebars.js way.

[everplays]

yeah, because in js arrays and objects are separable and handlebars.js decides only based on type of container but we can't do it in php.

anyways, I think @index + @key for Traversables will do the job.

On Sun, Feb 23, 2014 at 3:16 PM, Dmitriy S. Simushev < notifications@github.com> wrote:

In Handlebars.js even if an object has only integer keys, "@keyhttps://github.com/key" will be provided for it. Here is updated fiddle http://jsfiddle.net/75qha/1/

I think that if we provide for \Traversable instances more info ("@indexhttps://github.com/index"

• "@key https://github.com/key") it will be better than if we provide less info (only "@index https://github.com/index"). Only "@indexhttps://github.com/index" can be provided for arrays with sequential numeric keys. I believe it's Handlebars.js way.

— Reply to this email directly or view it on GitHubhttps://github.com/XaminProject/handlebars.php/pull/50#issuecomment-35829771 .

regards, behrooz

[everplays]

@JustBlackBird fixed it, like how he described it above (more info: #51). @d1rk, please give it a try and let us know if there's any problem. Thanks for the PR.