Dependency injection - passing responsibility for dependency to a Client?

Question

I have long standing argue about dependency injection and SOLID principles with my teammate. We both want to make an Exporter, to export data into various formats. My approach (in PHP):

class Exporter {
  public function __construct(private WriterContract $writer) 
  {
  }

  public function export(): void {
    // Do stuff and assign to $data
    $this->writer->write($data);
  }
}

class ExportController {
  public function __construct(private Exporter $exporter) {
  }

  public function route(Request $request): Response {
    $this->exporter->export();

    return Response(200, ['success!'])
  }
}

Teammates approach:

class Exporter {
  public function __construct(private WriterContract $writer) 
  {
  }

  public function export(): Data {
    // Do stuff and assign to $data
    return $data;
  }
}

class ExportController {
  public function __construct(private Exporter $exporter, private WriterContract $writer) {
  }

  public function route(Request $request): Response {
    $data = $this->exporter->export();
    $this->writer->write($data);

    return Response(200, ['success!'])
  }
}

I think none of approaches is really bad, they just have different pros and cons. My approach is easier to use for a client, but adds extra dependency to the Exporter class. Teammates approach doesn't add extra dependency, but next developer who comes after my teammate needs to better know how to use it, because writer is now his responsibility. And Writer dependency is in fact only moved to a Client, so I don't see a real benefit here.

What do you think? Is any of these two approaches cleaner or objectively better? Am I missing any other approach?

Answer 1

They are both terrible! First off PHP..... ..and don't get me started on space indentation!...

Jokes aside. I think what you are missing is a Converter class and some read and write classes.

eg

public class Exporter //NEW LINE!!
{
  public bool Export(id)
  {
     inData = this.InRepository.Read(id);
     outData = this.Converter.Convert(inData);
     this.OutRepository.Write(id,outData);
  }
}

Now each separate bit of logic has its own class and you can orchestrate them in the Exporter class, OR if you are happy with fat controllers, which I think in this case is totally acceptable, in a controller method.

Answer 2

This all depends on how tightly coupled the data structure is to the export format. If you cannot use the same data structure to export to different formats, then there is no point in separating this logic. For example, if you need to export to a spreadsheet or an XML file, and both export formats need different data structures, just keep all the logic in one class. Separating the logic for reading, transforming and exporting data only makes sense when you can substitute any of the readers, writers and transformers without affecting the data structures passed between the components.