Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow object keys #24

Merged
merged 2 commits into from
Feb 4, 2021
Merged

Allow object keys #24

merged 2 commits into from
Feb 4, 2021

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Feb 4, 2021

WDYT about this? Currently only array-key is supported as key type for all functions.

@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #24 (7e8f271) into 2.0.x-dev (5d1b992) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           2.0.x-dev       #24   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
===========================================
  Files              2         2           
  Lines             43        43           
===========================================
  Hits              43        43           
Impacted Files Coverage Δ
src/iterable-functions.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d1b992...7e8f271. Read the comment docs.

@bpolaszek
Copy link
Owner

Yeah, definitely 🙂

@bpolaszek bpolaszek merged commit 7da8363 into bpolaszek:2.0.x-dev Feb 4, 2021
@simPod
Copy link
Contributor Author

simPod commented Feb 4, 2021

So I can edit other functions too. However, this restricts only one read of output iterable as the value is of type Generator

I'm used to it as I use generators extensively but don't have overview out of my bubble.

@simPod simPod deleted the allow-keys branch February 4, 2021 13:29
@bpolaszek
Copy link
Owner

Indeed, forgot that. I have a fix in mind.

@bpolaszek
Copy link
Owner

bpolaszek commented Feb 4, 2021

Well, here's the fix I was thinking about:

function iterable_map(iterable $iterable, callable $map): iterable
{
    return new class ($iterable, $map) implements IteratorAggregate {
        private $iterable;
        private $map;

        public function __construct(iterable $iterable, callable $map)
        {
            $this->iterable = $iterable;
            $this->map = $map;
        }

        public function getIterator(): Traversable
        {
            foreach ($this->iterable as $key => $item) {
                yield $key => ($this->map)($item);
            }
        }
    };
}

It works, but basically this anonymous class is exactly what IterableObject already does. Problem: IterableObject calls iterable_map itself, so if we use it directly, we're going into spaghetti code.

What we could do is a refactoring, where functions would just be shortcuts to IterableObject::* methods (meaning we move map / filter logic into IterableObject methods), instead of the opposite:

function iterable_map(iterable $iterable, callable $map): iterable
{
    return iterable($iterable)->map($map);
}

WDYT?

@simPod
Copy link
Contributor Author

simPod commented Feb 4, 2021

Ah yes, we simply have to leverage IteratorAggregate and we're good to go.

@bpolaszek
Copy link
Owner

OK, I will perform the refactoring as soon as #25 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants