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

Make a distinction between writers and formatters #293

Open
leroy0211 opened this issue Apr 17, 2019 · 5 comments · May be fixed by #649
Open

Make a distinction between writers and formatters #293

leroy0211 opened this issue Apr 17, 2019 · 5 comments · May be fixed by #649

Comments

@leroy0211
Copy link

Feature Request

Right now the Writers are actually Formatters and Writers in one. It would be much nicer to have Formatters and Writers. For example:

Formatters

  • XmlFormatter
  • JsonFormatter
  • RSSFormatter

Writers

  • FileWriter
  • ResponseWriter
  • StreamedResponseWriter
  • FlysystemWriter

This way the exporter component can be used for more usecases

@greg0ire
Copy link
Contributor

Currently the typical usage is $writer = new CsvWriter('data.csv');
Maybe we could deprecate this in favor of $writer = new CsvFormatter(new FileWriter('data.csv')):

<?php
final class CsvWriter extends CsvFormatter
{
     public function __construct(
        string $filename,
        string $delimiter = ',',
        string $enclosure = '"',
        string $escape = '\\',
        bool $showHeaders = true,
        bool $withBom = false,
        string $terminate = "\n"
    ) {
        // trigger deprecation here
        return parent::__construct(
            new FileWriter($filename)
            $delimiter,
            $enclosure,
            $escape,
            $showHeaders,
            $terminate,
            $withBom
        );
    }
}

or the other way around with CsvWrite extends FileWriter

@leroy0211
Copy link
Author

leroy0211 commented Apr 18, 2019

You could also only implement the WriterInterface, and implement both new classes. So both Formatters and Writers can be considered final classes.

final class CsvWriter implements WriterInterface
{
   private $writer;
   private $formatter;

   public function __construct(
        string $filename,
        string $delimiter = ',',
        string $enclosure = '"',
        string $escape = '\\',
        bool $showHeaders = true,
        bool $withBom = false,
        string $terminate = "\n"
    ) {
          $this->writer = new FileWriter($filename);
          $this->formatter = new CsvFormatter(
               $delimiter, 
               $enclosure, 
               $escape, 
               $showHeaders, 
               $withBom, 
               $terminate
          );
    }

    public function open()
    {
         // implement logic
    }

    public function write(array $data)
    {
         // implement logic
    }

    public function close()
    {
         // implement logic
    }
}

@greg0ire
Copy link
Contributor

Even better! If you feel up to it please submit a pull request with only the CSVFormatter and the FileWriter, and necessary changes in other classes (like the Exporter maybe?)

@stale
Copy link

stale bot commented Jan 30, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Jan 30, 2020
@SonataCI SonataCI removed the wontfix label Jan 30, 2020
@core23 core23 added the stale label Feb 7, 2020
@core23 core23 added keep and removed stale labels Feb 14, 2020
@core23 core23 reopened this Feb 14, 2020
@phansys
Copy link
Member

phansys commented Oct 22, 2023

I agree. We should have a more consistent mechanism to compose the required combination between formatters and writers.

Currently, we have cases like #648, where we are using the source to provide arbitrary formats. At the same time, we also have dedicated formatters like FormattedBoolWriter, which are a more robust option IMO.
The problem is the lack of a configurable integration that allows the usage of the required formatters.

If I'm not misunderstanding the purpose of these formatters, we could replace the arbitrary options mentioned above by formatters like DateTimeFormatter, EnumFormatter, etc.

@phansys phansys linked a pull request Oct 23, 2023 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants