2
\$\begingroup\$

I am working on a PHP application that transforms data from the input CSV (JSON, TXT, or XML) into an output CSV (JSON or database, for example).

The output format differs from the input: some columns can be skipped, and some values can be converted (e.g. blue => 1, or 1.85 => 185). Also, the headers can have different names in the output file.

The parser can read the input file, the converter performs the transformations, and then the storage service writes files or inserts a record in the database.

I want to follow SOLID and DRY principles, and my software should be open for ongoing extension but closed to edits and modification.

I created a command (Symfony command), and here is the execute method:

protected function execute(InputInterface $input, OutputInterface $output): int
    {
        if (!$this->lock()) {
            $output->writeln('The command is already running in another process.');

            return Command::FAILURE;
        }

        // Get the fileInputPath argument and check if the file exists
        $fileInputPath = $input->getArgument('fileInputPath');

        $filesystem = new Filesystem();
        if (!$filesystem->exists($fileInputPath)) {
            throw new FileNotFoundException('File not found.');
        }

        // Get the fileInputPath extension and instantiate the right parser
        $fileInfo = new SplFileInfo($fileInputPath);
        $fileExt = $fileInfo->getExtension();

        $parser = sprintf('\\App\\Parsers\\%sParser', strtoupper($fileExt));
        $parserContext = new ParserContext(new $parser());
        $rows = $parserContext->parseData($fileInputPath);

        // For which customer do you want to import new data?
        $customer = $input->getOption('customer');

        $headersConverter = sprintf('\\App\\Converters\\%s\\HeadersConverter', $customer);
        $converterService = sprintf('\\App\\Services\\%s\\ConverterService', $customer);

        // Data conversion
        $columnsToSkip = [];
        $convertedRows = [];
        foreach ($rows as $x => $row) {
            $yy = 0; // Since I can skip some columns (e.g. Name), I need a new index to identify the columns
            foreach ($row as $y => $column) {
                if ($x === 0) { // convert headers
                    $convertedHeader = $headersConverter::convert($column);
                    if ($convertedHeader) {
                        $convertedRows[0][] = $convertedHeader;
                    } else {
                        $columnsToSkip[] = $y; // Skip columns that do not have a header conversion
                    }
                } else { // convert values
                    $currentColumn = null;
                    if (array_key_exists($yy, $convertedRows[0])) {
                        $currentColumn = $convertedRows[0][$yy];
                    }

                    if (!in_array($y, $columnsToSkip)) {
                        $convertedRows[$x][$y] = $converterService::convert($currentColumn, $column);
                        $yy++;
                    }
                }
            }
        }

        // Write on file or database
        $storageType = $input->getArgument('storageType');
        $fileOutputPath = $input->getArgument('fileOutputPath');

        $storage = StorageService::getStorageObjectFromStorageType($storageType, $fileOutputPath);

        $storageContext = new StorageContext($storage);
        $storageContext->saveData($convertedRows);

        // Print output
        $output->writeln([
            'Command run successfully!',
        ]);

        if (in_array($storageType, ['csv', 'json'])) {
            $output->writeln([
                sprintf('A new %s file has been created in the data folder.', $storageType)
            ]);
        }

        return Command::SUCCESS;
    }

Based on the input file extension, the following line can decide which parser to use:

$parser = sprintf('\\App\\Parsers\\%sParser', strtoupper($fileExt));

Depending on the name of the customer (an option of the command), I can convert headers and values with different classes:

$headersConverter = sprintf('\\App\\Converters\\%s\\HeadersConverter', $customer);
$converterService = sprintf('\\App\\Services\\%s\\ConverterService', $customer);

I rely on static methods. Maybe there is a better way to do it. Can you suggest me something, please? Also, the execute method is very procedurally written and may contain too much logic.

Any review is very welcome. If you need more code, I am happy to share it with you.

\$\endgroup\$
2
  • \$\begingroup\$ This function is starting to get Too Long. Maybe that foreach loop is a good candidate for your first Extract Helper ? \$\endgroup\$
    – J_H
    Feb 27 at 23:49
  • 1
    \$\begingroup\$ Thanks for your input. I agree, I moved it into its RowsConverter class. This is now that part: $convertedRows = (new RowsConverter(new $headersConverter(), new $converterService())) ->convert($rows); \$\endgroup\$ Feb 28 at 14:44

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.