Suggestion: use Objects instead of arrays

[LauLaman]

I think using arrays for mapping is a bad practice. It makes more sense to create an object for this:

final class PaymentMap
{
    const FIELD_AMOUNT = 'amount';
    const FIELD_COUNTERPARTY_ALIAS = 'counterparty_alias';
    const FIELD_DESCRIPTION = 'description';
    const FIELD_ATTACHMENT = 'attachment';

    private $amount;

    private $pointer;

    private $description;

    private $attachments;

    public function __construct(Amount $amount, Pointer $pointer, $description)
    {
        //... the usual stuff ...//
    }

    public function addAttachment(Attachment $attachment): void
    {
        $this->attachments[$attachment->getId()] = $attachment;
    }

    public function getAmount(): Amount
    {
        return $this->amount;
    }

    public function getAsArray(): array
    {
        return [
            self::FIELD_AMOUNT => $this->amount,
            self::FIELD_COUNTERPARTY_ALIAS => $this->pointer,
            //... Well you get the idea ...// 
        ];
    }
}

$paymentMap = new PaymentMap(
    new Amount('10.00', 'EUR'),
    new Pointer('EMAIL', 'api-guru@bunq.io'),
    This is a generated payment',
];

Payment::create($bunqApiContext, $paymentMap, $userId, $monetaryAccountId);

public static function create(ApiContext $apiContext, PaymentMap $paymentMap, $userId, $monetaryAccountId, array $customHeaders = [])
    {
        $apiClient = new ApiClient($apiContext);
        $response = $apiClient->post(
            vsprintf(
                self::ENDPOINT_URL_CREATE,
                [$userId, $monetaryAccountId]
            ),
            $paymentMap->getAsArray(),
            $customHeaders
        );

        return static::processForId($response);
    }

Using this approach you restrict the programmer to fill in all the required fields, and prevent making invalid API calls.

It adds more structure to the project and gives the ability to run better tests :)

you can even unit test the whole thing now:

/**
 * @small
 */
final class PaymentMapTest extends PHPUnit_Framework_TestCase
{
    /** @test */
    public function canCreateValidPaymentMap()
    {
        $amount = new Amount(10.00, 'EUR');
        $pointer= new Pointer('EMAIL', 'jon.doe@example.com');
        $description = 'payment description';
        $paymentMap = new PaymentMap($amount, $pointer, $description);

        self::assertSame($amount, $paymentMap->getAmount());
        self::assertSame($pointer, $paymentMap->getPointer());
        self::assertSame($description, $paymentMap->getDescription());
    }

    /** @test */
    public function canCreateValidPaymentMapParsesValidArray()
    {
        // duplicate code maybe extract it into a method?
        $amount = new Amount(10.00, 'EUR');
        $pointer= new Pointer('EMAIL', 'jon.doe@example.com');
        $description = 'payment description';
        $paymentMap = new PaymentMap($amount, $pointer, $description);

        self::assertEquals(
            [
                PaymentMap::FIELD_AMOUNT => $amount,
                PaymentMap::FIELD_COUNTERPARTY_ALIAS => $pointer,
                //... Well you get the idea ...//
            ],
            $paymentMap->getAsArray()
        );
    }

    /** @test */
    public function canAddAttachments()
    {
        // duplicate code maybe extract it into a method?
        $amount = new Amount(10.00, 'EUR');
        $pointer= new Pointer('EMAIL', 'john.doe@example.com');
        $description = 'payment description';
        $paymentMap = new PaymentMap($amount, $pointer, $description);

        $attachment = new Attachment();
        $paymentMap->addAttachment($attachment);

        // ... Well you get the idea ...//
    }
}

[OGKevin]

This sounds and looks interesting 🤔, what do you think @dnl-blkv @andrederoos

[dnl-blkv]

@LauLaman Thanks for the suggestion. We definitely have it on our list since a day before the first line of SDK code was written. However, it is quite a big change. Also, due to specifics of our SDK building process it is not something community can build.

We will do it, but later on. Closing for now :)