RFC: Proposal for next major

[PowerKiKi]

Model closer to spec

While maintaining this lib in the past few months, I noticed its model is sometimes a simplification of the specification. While it makes using the lib somewhat easier, it also prevent to access some data that exists in the XML but cannot be modelized (without twisting even more the model).

In order to fix that I'd like to introduce breaking changes by re-modeling the entire thing to stick much closer to the specification (while retaining a few simplification if it does not prevent supporting more data).

So instead of this:

// access to BkToCstmrDbtCdtNtfctn->Ntfctn->Ntry->NtryDtls->TxDtls->AmtDtls->TxAmt->Amt
$amount = $message->getRecords()[0]->getEntries()[0]->getTransactionDetail()->getAmountDetails();

We would get something closer to this:

// access to BkToCstmrDbtCdtNtfctn->Ntfctn->Ntry->NtryDtls->TxDtls->AmtDtls->TxAmt->Amt
$amount = $message->getRecords()[0]->getEntries()[0]->getTransactionDetails()[0]->getAmountDetails()->getTransactionAmount();

// New possibility: access to until-now hidden data InstdAmt !
// access to BkToCstmrDbtCdtNtfctn->Ntfctn->Ntry->NtryDtls->TxDtls->AmtDtls->InstdAmt>Amt
$amount = $message->getRecords()[0]->getEntries()[0]->getTransactionDetails()[0]->getAmountDetails()->getInstructedAmount();

Notice how new data are available because we follow more closely the structure of the spec. And notice also that the singular getTransactionDetail() disappear entirely.

This would also mean to rename a few things to be closer to spec. Sometimes the spec has a lot of different terms, but in the end all of those terms exists for a reason. Renaming things to be closer to the spec would help developers understand the spec better and thus allow them to better choose which data they should access.

The limitations described in 5de970a would also be lifted.

Readonly model

Since we would be introducing breaking changes, I would also like to explore the possibility to have an entirely readonly model, based on public properties, instead of getters. So the final result would be:

// access to BkToCstmrDbtCdtNtfctn->Ntfctn->Ntry->NtryDtls->TxDtls->AmtDtls->TxAmt->Amt
$amount = $message->records[0]->entries[0]->transactionDetails[0]->amountDetails->transactionAmount;

The reasoning being that, first of all, the library users are never meant to update data, because the library cannot write back to XML. And keeping only readonly properties reduce maintenance burden by drastically reducing code quantity, and avoiding type declaration duplications.

This is a quick sample of what I had in mind:

PHP support

All of this would most likely target PHP 8.2, because it brings the nice-to-have readonly classes. So we might drop support for PHP 8.1 a bit before it becomes EOL, but not by much. I guess that should be fine

RFC ?

@frederikbosch, @localheinz, and everybody else, does all of this sound reasonable to you ? would you foresee trouble further down the line ? or did you have different ideas altogether ?

[frederikbosch]

I think this is very reasonable. I would also love to see Options and Collections in this library, because it guards against missing elements, empty arrays etc. We have many places where you can hit PHP warnings with the current state of the library. But that would require adding a dependency to provides these elements.

[PowerKiKi]

By Collection, do you mean something like https://www.php.net/manual/en/class.arrayobject.php ? is your goal to throw an exception if trying to access an offset that does not exist ?

If we want to avoid adding a dep for Collection, we could type as iterable to prevent direct offset access and always force usage via foreach... but I'm not entirely sure this would make the API pleasant to use... ? 🤔

I assume "Options" is referring to something like https://github.com/schmittjoh/php-option. I agree with the end-goal, to force to consider null cases, but I am bit unsure about the way to do it. If we introduce a dependency for that, I feel like it could increase friction for our users since we would force a new way of thinking on them. In my own projects I rely extensively on PHPStan to force me to think about those kind of cases. So far it's been "good enough" for me. This approach let our users make their own choice, PHPStan, Psalm, wrapping our API with Options. I see that as an advantage... but maybe I am not experienced enough with Options to figure out that it is the superior alternative ? 🤔

[frederikbosch]

->bankTransactionCode->unwrapOrElse(fn () => );
->transactionDetails->first()->andThen();
->transactionDetails->first()->mapOrElse();

try {
  $result = $camt->transactionDetails->first()->mapOrElse()->unwrapOrElse();
} catch (NoneException) (
}

So you have:

public Collection $transactionDetails;

And inside Collection:

public function first(): Option
{
  if ($this->innerArray === []) {
    return Option:none();
  }
  return Option::some($this->innerArray[0]);
}

Inside Collection and Option you implement all kinds of methods that can help to change the content of the collection: e.g. map it to an object of your own library, filter it, or in case of Option you provide an alternative value if it is none.

Because our library has so many optionalities and collections, it would help to work with it if we provide classes that help the client developer to consume data from the camt statements.

[GeNTooFReaK]

As someone who is using this parser right now I would love to see this proposal become reality soon, as I'm needing the InstdAmt currently and can't get to it with the current version. Making the class readonly with public properties would also be my favored way.

[PowerKiKi]

I am still willing to work on a purely readonly model, but I am still unconvinced about Options and Collections as suggested by Frederik. I think it would make using the lib much harder for people unfamiliar with those concepts. And people who care about robust code can use PHPStan.

@frederikbosch is it ok with you to go on with this RFC, but without Options and Collections ? or is that a blocker for you ?

[frederikbosch]

Please go ahead and ignore my suggestion, I am fine with anyone improving the library.

[GeNTooFReaK]

I do like Options and Collections, as I love Rust where these concepts are essential parts of the language, but I agree with @PowerKiKi that most PHP devs aren't familar with that and would struggle to use them. So me myself wouldn't use them in a public PHP lib which many people should be able to use easily.

[frederikbosch]

@GeNTooFReaK It’s where I got the inspiration from. This library would be particularly suited for it.

[frederikbosch]

Remove Genkgo\Camt\Decoder\DateDecoderInterface as it is very likely that there is no need for it, see discussion in #173.