CODEWITHSUNDEEP
symfonydesign-patternsdependency-injection
user2268997
user2268997

Reputation: 1391

Defining Symfony Services to maximize maintainability

I'm working on a big domain, for which maintainability is very important.
There are these general workers called ExcelHandlers that implement ExcelHandlerInterface (more on the interface in the ideas section) and basically get an UploadedFile as their input, upload them wherever they want and return the read data as an associative array. Now I have created this base class ExcelFileHandler which does all of these tasks for all excel files given two arguments:
1. The Directory to upload the file
2. the mapping of the excel columns to the indexes of the associative array.
Some ExcelHandlers might have to extend the ExcelFileHandler and do some more processing, in order to get the associative array of data.
The UploadedFile is always passed to the ExcelHandler from the controller.
Now here is the question. Given the generic structure of the ExcelFileHandler how should I define services for specific ExcelHandlers given that some only differ with the original one in the directory to upload the file and the mapping array.
My Ideas:

1. The first approach involves giving the directory and the mapping as the function arguments to ExcelHandleInterface::handle this will make the prototype something like handle(UploadedFile $file, array $mapping, $dir), $mapping and $dir are given to the function as arguments and passed to the handler by the controllers which has the parameters as constructor injections.

2.1 Defining the prototype of handle to be handle(UploadedFile $file), this would require the ExcelHandlers to have knowledge of $dir and $mapping. $dir will always be injected from the constructor. 2.1.1 Foreach individual ExcelHandler in the application, define a separate class e.g: UserExcelHandler, ProductExcelHandler, .... Which extend the ExcelFileHandler and leaves us again with two choices.
2.1.1.1 inject $mapping from outside. e.g: 

// in the child class
public function __construct($dir, $mapping){
  parent::__construct($dir, $mapping);
}

2.1.1.2 define $mapping in the constructor of the child class. e.g: 

// in the child class
public function __construct($dir){
  $mapping = array(0 => 'name', 1 => 'gender');
  parent::__construct($dir, $mapping);
}

2.1.2 Not to create a class for each separate ExcelHandler and instead define the ExcelFileHandler as an abstract service and decorate with the right parameters to get the concrete ExcelHandler Service with the desired functionality, obviously ExcelHandlers with custom logic must be defined seperately, and to create a uniform code base, $mapping will always be injected from the Container in this case.

In your experience, what other paths can I take and which ones yield better results in the long term?

Upvotes: 0

Views: 70

Answers (1)

Marius Balčytis
Marius Balčytis

Reputation: 2651

First of all, it seams as you've already put two separate things into one.

Uploading a file and reading it's contents are two separate concerns, which can change separately (like you said, $directory and $mapping can change case-by-case). Thus I would suggest to use separate services for uploading and reading the file.

See Single responsibility principle for more information.

Furthermore, due to very similar reasons, I would not recommend base classes at all - I'd prefer composition over inheritance.

Imagine that you have 2 methods in your base class: upload, which stores file to a local directory, and parse, which reads excel file and maps columns to some data structure.

  1. If you need to store file in a remote storage (for example FTP), you need to override upload method. Let's call this service1.
  2. If you need to parse file differently, for example combining data from 2 sheets, you need to override parse method. Let's call this service2.
  3. If you need to override both of these methods, while still being able to get service1 and service2, you're stuck, as you'll need to copy-and-paste the code. There's no easy way to use already written functionality from (1) and (2).

In comparison, if you have interface with upload method and interface with parse method, you can inject those 2 separate services where you need them as you need them. You can mix any implementations of those already available. All of them are already tested and you do not need to write any code - just to configure the services.

One more thing to mention - there is absolutely no need to create (and name) classes by their usage. Name them by their behaviour. For example, if you have ExcelParser, which takes $mapping as an argument to a constructor, no need to call it UserExcelParser if the code itself has nothing to do with users. If you need to parse data from several sheets, just create SheetAwareExcelParser etc., not ProductExcelParser. This way you can reuse the code. Also correct naming lets understand the code more easily.

In practice, I've seen when function or class is named by it's usage, and then it's just used in another place with no renaming, refactoring etc. These cases are really not what you're looking for.

Service names (in another words concrete objects, not classes), can of course be named by their purpose. You just configure them with any required functionality (single vs separate sheets etc.)

To summarize, I would use 2.1.2 above all other of your given options. I would inject $dir and $mapping via DI in any case, as these do not change in runtime - these are configuration.

Upvotes: 2

Related Questions