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

Added implode method #10

Merged
merged 2 commits into from
Oct 30, 2018
Merged

Added implode method #10

merged 2 commits into from
Oct 30, 2018

Conversation

maxalmonte14
Copy link
Member

This implementation is directly taken from #8 issue.

Now all CollectionAbstract children have an implode method available.

A call in a ColumnCollection should be as the following:

$columnCollection->implode(',', function ($column) {
    return $column->getLabel();
});

And, in a RowCollection:

$rowCollection->implode("\n", function ($row) {
    return implode(',', $row);
});

Tests are included, but like we don't test abstract classes I made it with the CollumnCollection class.

@maxalmonte14 maxalmonte14 changed the base branch from master to develop October 25, 2018 18:39
@maxalmonte14
Copy link
Member Author

Interesting, this PR doesn't pass the CI, I have the following output in my development environment, even before implementing the feature, so I don't see what's is missing.

 ------------------------------------------------------------------------------------------------------------------------------------
  Error
 ------------------------------------------------------------------------------------------------------------------------------------
  Ignored error pattern #Result of method [a-zA-Z]+::[a-zA-Z]+\(\) \(void\) is used.# was not matched in reported errors.
  Ignored error pattern #Method [a-zA-Z\\]+::[a-zA-Z]+\(\) should return bool but returns void.# was not matched in reported errors.
 ------------------------------------------------------------------------------------------------------------------------------------

@maxalmonte14
Copy link
Member Author

I'm a newbie in this PHPStan thing but if I remove the whole parameters key in the phpstan.neon file I have no errors 😅

@maxalmonte14
Copy link
Member Author

maxalmonte14 commented Oct 26, 2018

I also think RowCollection class should store Row objects instead of simple arrays, this way we could implement the __toString magic method and return all the properties separated by ,, like so:

Row class:

class Row
{
    public function __toString()
    {
        return implode(',', $this->items);
    }
}

And then call

$rowCollection->implode("\n", function ($row) {
    // return implode(',', $row);
    return $row;
});

@willishq
Copy link
Collaborator

I just took the ignore errors thing out because it was complaining that the errors I told it to ignore were not found when running phpstan.

@willishq willishq merged commit 13f51a0 into develop Oct 30, 2018
@willishq willishq deleted the feature/implode_collection branch October 30, 2018 11:37
@maxalmonte14
Copy link
Member Author

That was my first thought, well, now we can focus on the exporters repo 👍

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.

None yet

2 participants