5
\$\begingroup\$

I'm trying to build an API wrapper class, that sends requests in a JSON format. the sample request format is something like this:

{
  "items": [
    {
      "brand": "brand",
      "idCategory": "idCategory",
      "idItem": "idItem",
      "attributes": [
        {
          "idUda": "idUda",
          "price": "price"
        },
        {
          "idUda": "idUda",
          "price": "price"
        }
      ],
      "description": "description",
      "skus": [
        {
          "price": {
            "offer": "offer",
            "default": "default"
          },
          "stock": {
            "available": "available",
            "amount": "amount"
          },
          "gtin": "gtin",
          "images": [
            "image",
            "image"
          ],
          "idSku": "idSku",
        }
      ]
    }
  ]
}

My first thought was to create an object for each element of the JSON request that is an array, something like this:

        $attr1 = new Attribute;
        $attr1->setIdUda('xxx')
        ->setPrice('99999');

        $price = new Price;
        $price->setOffer('teste')
        ->setDefault('99.99');

        $stock = new Stock;
        $stock->setAvailable(true)
        ->setAmount('9999');

        $product = new Product;
        $product->setBrand('Apple')
        ->setIdCategory('500')
        ->setIdItem('9999')
        ->setAttributes($attr1)
        ->setDescription('nice product')
        ->setSkus($price);
        ->setSkus($stock);

And each one these objects have a toArray() method, that returns an associative array, e.g.:

class Price {
        
        //getters and setters....
            
            function toArray() {
                return array(
                    'offer' => $this->offer,
                    'default' => $this->default
                )
            }
        }

Inside of the Product class all these attributes setters are grouped to form an array so I can convert it to the JSON format later.

class Product {
        //..... 
    public function setAttributes(Attribute $attributes) {

        $this->attributes['attributes'][] = $attributes->toArray();
                    return $this;
    }
}

But it still feels like it's highly coupled, because i have to set the key of each one of these arrays, in order to form the JSON data.

What would be the "best" approach to make this code easier to maintain? Should I use classes only for some attributes?

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

There is no problem modelling the request with classes representing each part of the request and a class that aggregates them.

One thing to note here, those classes adhere to builder pattern - builder of JSON. Maybe you don't know that there is an interface \JsonSerializable which would communicate the intent of the classes much better then a toArray() method. Although many arrays can be deemed JSON serializable, its not always true. Where as JSON serializable object is always serializable to JSON and it makes it clear that the JSON serializability is its purpose...

Then I quite don't understand this piece:

        $product
        ->setSkus($price);
        ->setSkus($stock);

You set skus to a Price object, then overwrite with Stock object? Given this piece of data:

"skus": [
        {
          "price": {
            "offer": "offer",
            "default": "default"
          },
          "stock": {
            "available": "available",
            "amount": "amount"
          },
          "gtin": "gtin",
          "images": [
            "image",
            "image"
          ],
          "idSku": "idSku",
        }
      ]

I would expect that there is one more object between Product and Stock/Price object. Maybe a Sku object:

$product->addSku((new Sku())->setPrice($price)->setStock($stock));

And I wouldn't also see problem adding methods for more convenient work (that would instantiate the parts classes):

$sku = new Sku();
$sku->setPrice('offer', 'default');
$sku->setStock('available', 'amount')

instead of:

$sku = new Sku();
$sku->setPrice(new Price('offer', 'default'));
$sku->setStock(new Stock('available', 'amount'));

After all, those classes are not meant to represent the request, they represent a builder of a JSON request. And builder should make it as easy to build the target as possible....

EDIT: you were worried about coupling. I don't think this brings in any more coupling then necessary. The API uses specific keys in the request and these will never change, unless the API introduces BC break. But then maybe it was not a very good API structure... Anyway it wouldn't hurt to have those keys defined as constants of the builder classes, so you don't have to repeat string literals all over the place... if nothing it makes it easier to find all occurrences of a key.

Chances are that there are multiple ways the API could have been designed, yet retaining the same capabilities. And this means that there are also multiple ways to model the request with PHP objects. But you should prefer to have this 1:1. The API designer was probably aware of the multiple ways and chosen the one he chosen for a reason. And you should follow that reasoning...

Well this statement might have seem quite abstract, so let me complete it with an example. Lets say we have API structure:

{
  "scopeA": {
    "configA1": "valueA1"
  },
  "scopeB": {
    "configB1": "valueB1"
  }
}

but we can also model it as this structure:

{
  "scopeAconfigA1": "valueA1",
  "scopeBconfigB1": "valueB1"
}

but the first structure is the API structure, and so the PHP model should also have the first structure. Simply following the same reasoning why the API designers chose the first structure. Whatever reasoning it may have been...

\$\endgroup\$
8
  • \$\begingroup\$ thanks for the reply. about the SKU object, i created a SKU interface, so all the SKU Items can implement the toArray method. \$\endgroup\$ Nov 3, 2019 at 22:08
  • 1
    \$\begingroup\$ @NBAYoungCode No dont create toJson method. toArray() is actualy fine, but it could be finer if you used the JsonSerializable instead. It offers jsonSerialize() which would just be the same implementation as your toArray(), just different name. And it can then be used like this json_encode($object), where the $object implements JsonSerializable. Not really different from json_encode($object->toArray()), but the intent is more clear.... \$\endgroup\$
    – slepic
    Nov 4, 2019 at 6:35
  • 1
    \$\begingroup\$ @NBAYoungCode No i dont think you should create ProductJSON and Product class separately. It's actualy the ProductJSON class what you have, but it should not be a problem to just call it Product, because in the context, probably everything is json.... Actualy the most precise naming would be ProductJsonRequestBuilder, but depends on how far you wanna go. Just Product would be pretty fine IMO, given the context ofc.. \$\endgroup\$
    – slepic
    Nov 4, 2019 at 6:37
  • 1
    \$\begingroup\$ @NBAYoungCode you might start considering to split it only if you have multiple apis with the same structure, but each using different keys. But thats probably never gonna happen. The keys are like constants and that's why it may be good to define them as constants of the classes as well. It's not really something that will ever change. And if it will, it will be BC break anyway - both on the API and library side... \$\endgroup\$
    – slepic
    Nov 4, 2019 at 6:39
  • 1
    \$\begingroup\$ @NBAYoungCode Yea, you cannot avoid the keys, they are part of the request. But only those builder classes need to know and it should be static constants of the bulder classes. It can even be private to them, since the consumers of the builder objects need not worry about the keys at all... Btw if you are satisfied with my answer, please consider marking it as accepted :) \$\endgroup\$
    – slepic
    Nov 5, 2019 at 17:51

Your Answer

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

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