search

sprain / php-swiss-qr-bill

A PHP library to create Swiss QR bills | QR-Rechnung in PHP erstellen
MIT License
274 stars 79 forks source link

Feature Proposal: Serialize and Unserialize of a QR-string #36

Open kohlerdominik opened 4 years ago

kohlerdominik commented 4 years ago

My apologies for a lot of noise from me the last few days. We just started working on the QR-Bill implementation.

I am currently writing our specification of the feature, and came to the conclusion, that it could make sense to store and restore the QR-Code content instead of creating it from data every time.

Since PHP 7.4 we could wrap it in __serialize and __unserialize, but I think that's not what it's intentional use is (for me php-serialize is more technical, while this seems Domain Knowledge).

Therefore, I think getContentString() and parseContentString($string) could make more sense.

I would be happy to work out a PR to add this on occasion. But maybe there's already a similar feature implemented I'm missing here? Or are there some specific reason that it's not supported in the first place?

sprain commented 4 years ago

My apologies for a lot of noise from me the last few days. We just started working on the QR-Bill implementation.

No problem. I am glad the library is helpful for you and feedback is always welcome.

I am currently writing our specification of the feature, and came to the conclusion, that it could make sense to store and restore the QR-Code content instead of creating it from data every time.

What already works is to get the actual contents of the qr code image with $qrBill->getQrCode()->getText();. This contains all necessary information as this is what the bank gets as well. I can see the benefit of creating the whole domain model by injecting such a string and then (re)create outputs from that. Such an approach would also allow to use the library to read data from existing qr bills (for example as a receiver of bills).

I'd suggest an integration like this:

// Serialize
$existingQrBillString = $qrBill->getQrString(); // internally doing $qrBill->getQrCode()->getText();

// Deserialize
$newQrBill = QrBill::createFromQrString($existingQrBillString); // this needs to be built

Would this cover your use case or did you have something else in mind?

kohlerdominik commented 4 years ago

Yes, this covers exactly my idea here. The shortcut getQrString() makes sense to me, as it seems more obvious on the QrBill class. But createFromQrString could be quite some work.

From your comment I didn't understand if you suggest how I should implement it, or if you plan to add the feature yourself. As said, i can try to work on it, but i can't estimate when it would be ready, as i neither know how long it will take nor how much time i can spend on it within the next weeks.

sprain commented 4 years ago

From your comment I didn't understand if you suggest how I should implement it, or if you plan to add the feature yourself. As said, i can try to work on it, but i can't estimate when it would be ready, as i neither know how long it will take nor how much time i can spend on it within the next weeks.

I cannot tell whether I will have time for this in the next few weeks. So I suggest, we leave it pending here and if anybody wants to start working on it, they hopefully will let us know :)