ZATCA icon indicating copy to clipboard operation
ZATCA copied to clipboard

Enable file save in render method

Open ayaseensd opened this issue 2 years ago • 2 comments

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Adding file save feature

What is the current behaviour? (You can also link to an open issue here)

  • the current behaviour it recieve array of options without argument for saving file which provided by chillerlan/php-qrcode

What is the new behaviour? (You can also link to the ticket here)

  • I edit render method of GenerateQrCode class, to accept mixed arguments, I only process on two arguments, if each of one arguments is string I consider it file path, otherwise I consider it array argument for QROptions class.
  • arrangement of two arguments ignored, and more than 2 arguments also ignored.
  • I use this way to make render call clean and to avoid breaking of previous behaviour.

Does this PR introduce a breaking change?

  • No

ayaseensd avatar Oct 17 '22 07:10 ayaseensd

Why not add another optional parameter to the render function?

public function render(array $options = [], string $file = null): string {
        $options = new QROptions($options);
        return (new QRCode($options))->render($this->toBase64(), $file ?? null);
}

I think its more IDE friendly and most importantly cleaner.

ali-alharthi avatar Oct 21 '22 12:10 ali-alharthi

@ali-alharthi I first implemented this approach, which is similar to chillerlan/php-qrcode implementation in that I intended to run render method unconditionally params order, so render($options), render($file),render($file,$options) or render($options,$file).

but I think you are right.

ayaseensd avatar Oct 23 '22 07:10 ayaseensd

@ayaseensd @ali-alharthi thank you for your contributions

salkhwlani avatar Nov 04 '22 22:11 salkhwlani