1
\$\begingroup\$

I have 2 files

wholesaler_a.csv

id;ean;manufacturer;product;description;packaging product;foo;packaging unit;amount per unit;items on stock (availability);warehouse
12345600001;23880602029774;Drinks Corp.;Soda Drink, 12 * 1,0l;Lorem ipsum usu amet dicat nullam ea;case 12;bar;bottle;1.0l;123;north

and wholesaler_b.json

{
  "data": [
    {
      "PRODUCT_IDENTIFIER": "12345600001",
      "EAN_CODE_GTIN": "24880602029766",
      "BRAND": "Drinks Corp.",
      "NAME": "Soda Drink, 12x 1L",
      "PACKAGE": "case",
      "ADDITIONAL_INFO": "",
      "VESSEL": "bottle",
      "LITERS_PER_BOTTLE": "1",
      "BOTTLE_AMOUNT": "12"
    }
  ]
}

The goal is to convert them to the following JSON format

{
  "id": "",
  "gtin": "",
  "manufacturer": "",
  "name": "",
  "packaging": "",
  "baseProductPackaging": "",
  "baseProductUnit": "",
  "baseProductAmount": "",
  "baseProductQuantity": ""
}

Here is my implementation:

<?php


namespace Egy\Test;


abstract class AbstractProductAdapter implements ProductAdapterInterface
{
    protected $data;

    public function getMappedKeys(): array
    {
        return [
            $this->getId() => DataAdapter::FIELD_ID,
            $this->getGtin() => DataAdapter::FIELD_GTIN,
            $this->getManufacture() => DataAdapter::FIELD_MANUFACTURE,
            $this->getName() => DataAdapter::FIELD_NAME,
            $this->getPackaging() => DataAdapter::FIELD_PACKAGING,
            $this->getBaseProductPackaging() => DataAdapter::FIELD_BASE_PRODUCT_PACKAGING,
            $this->getBaseProductUnit() => DataAdapter::FIELD_BASE_PRODUCT_UNIT,
            $this->getBaseProductAmount() => DataAdapter::FIELD_BASE_PRODUCT_AMOUNT,
            $this->getBaseProductQuantity() => DataAdapter::FIELD_BASE_PRODUCT_QUANTITY
        ];
    }

    public function __construct(DataProvider $dataProvider)
    {
        $this->data = $dataProvider->getProducts();
    }

    public function mapData(): array
    {
        $mappedData = [];

        foreach ($this->data['data'] as $item) {
            $mappedData[] = array_combine(array_merge($item, $this->getMappedKeys()), $item);
        }

        return $mappedData;
    }
}

<?php


namespace Egy\Test;


interface DataProvider
{
    public function getProducts();
}
<?php


namespace Egy\Test;


class CsvData implements DataProvider
{
    protected $file;

    public function __construct(string $file)
    {
        $this->file = $file;
    }

    public function getProducts(): array
    {
        $rows = array_map(function ($value) {
            return str_getcsv($value, ";");
        },
            file($this->file)
        );

        $header = array_shift($rows);

        $data = [];
        foreach ($rows as $row) {
            $data['data'][] = array_combine($header, $row);
        }

        return $data;
    }
}

<?php


namespace Egy\Test;


class CsvProductAdapter extends AbstractProductAdapter
{
    const FIELD_ID = 'id';
    const FIELD_GTIN = 'ean';
    const FIELD_MANUFACTURE = 'manufacturer';
    const FIELD_NAME = 'product';
    const FIELD_PACKAGING = 'packaging product';
    const FIELD_BASE_PRODUCT_PACKAGING = 'packaging unit';
    const FIELD_BASE_PRODUCT_UNIT = 'amount per unit';
    const FIELD_BASE_PRODUCT_AMOUNT = 'items on stock (availability)';
    const FIELD_BASE_PRODUCT_QUANTITY = 'warehouse';

    public function getId(): string
    {
        return self::FIELD_ID;
    }

    public function getGtin(): string
    {
        return self::FIELD_GTIN;
    }

    public function getManufacture(): string
    {
        return self::FIELD_MANUFACTURE;
    }

    public function getName(): string
    {
        return self::FIELD_NAME;
    }

    public function getPackaging(): string
    {
        return self::FIELD_PACKAGING;
    }

    public function getBaseProductPackaging(): string
    {
        return self::FIELD_BASE_PRODUCT_PACKAGING;
    }

    public function getBaseProductUnit(): string
    {
        return self::FIELD_BASE_PRODUCT_UNIT;
    }

    public function getBaseProductAmount(): string
    {
        return self::FIELD_BASE_PRODUCT_AMOUNT;
    }

    public function getBaseProductQuantity(): string
    {
        return self::FIELD_BASE_PRODUCT_QUANTITY;
    }
}

And same implementation for JSON format

Could you please review my code

\$\endgroup\$
0
\$\begingroup\$

I recommend fewer loops in CsvData's getProducts()

public function getProducts(): array
{
    $lines = file($this->file);
    $header = str_getcsv(array_shift($lines), ';');

    return [
        'data' => array_map(
            function($line) use ($header) {
                return array_combine(
                    $header,
                    str_getcsv($line, ';')
                );
            },
            $lines
        )
    ];
}

Or if you are on PHP7.4 or higher, use an arrow function for less verbose syntax:

public function getProducts(): array
{
    $lines = file($this->file);
    $header = str_getcsv(array_shift($lines), ';');

    return [
        'data' => array_map(
            fn($line) => array_combine($header, str_getcsv($line, ';')),
            $lines
        )
    ];
}

I'm not sure that I am sold on the array_merge()-array_combine() technique in mapData(). (Just quietly, I think array_replace() is more semantically appropriate versus array_merge() -- though the effect is the same.) The use of array_combine() -- a function which is rather strict regarding its data requirements -- in getProducts() is more-or-less forgivable assuming the data sources are reliably valid. However, in mapData(), there may be some development or change to file contents which is not covered by your getMappedKeys(). This makes me think that there should be some conditions to throw exceptions in the class. I also wonder if instead of individual const declarations, I might use a lookup array to perform the translation. Maybe something for you to ponder, as I am not settled on my opinion yet.

Finally, I don't see any benefit in nesting the relevant data inside of $data['data']. Always try to design the least complex data structures possible. In other words, strip out the needless first level as soon as possible.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.