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

add trait for row to string converting without file #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

turkishjoe
Copy link

I need convert my row to csv format without creating file. Your function rowToStr was usefull for me.

@tomasfejfar
Copy link
Contributor

tomasfejfar commented May 1, 2019

First, thank you for the contribution.

I don't think trait is a good fit for this, though. I think this could be a normal class (CsvFormatter) with pure function (having all the row, delimiter, enclosure, etc. as parameters) that can have proper test. That class can then be used by the means of composition in the CSV file class...

@turkishjoe
Copy link
Author

turkishjoe commented May 1, 2019

Trait is not best solution for it. But i put for legacy. Method rowToStr should be removed from CsvWriter.
For you CsvFormater we need methods setDelimiter, validateDelimiter and etc. I can extend AbstractCsvFile, but we have a filename(resourse) as constructor argument.

It was my case. I needed only csv row, but not in file. My fast solution was. I want make a normal solution. I will try do it.

class VirtualCsvWriter extends CsvWriter
{
    public function __construct(
        $file = '',
        $delimiter = self::DEFAULT_DELIMITER,
        $enclosure = self::DEFAULT_ENCLOSURE,
        $lineBreak = "\n"
    ) {
        parent::__construct($file, $delimiter, $enclosure, $lineBreak);
    }

    protected function setFile($file)
    {
    }
}

First, thank you for the contribution.

I don't think trait is a good fit for this, though. I think this could be a normal class (CsvFormatter) with pure function (having all the row, delimiter, enclosure, etc. as parameters) that can have proper test. That class can then be used by the means of composition in the CSV file class...

@tomasfejfar
Copy link
Contributor

You are right that we'd need to reimplement all the input parameters validation in such class. This will probably need some further refactoring. Maybe adding a ValueObject holding validated settings that could be passed along - something like CsvFormatParameters. That way the formatter would only need this object and the same object could be used in CsvFile and whereever else we wanted.

Let me think about that...

@tomasfejfar tomasfejfar self-assigned this May 2, 2019
@turkishjoe
Copy link
Author

Yeah! It is good idea. Also we can use something like Input/Output interface and read/write csv rows from stdin/out or file. I also think about it.

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.

3 participants