Inconsistency in $foregroundColor (string and array)

[tacman]

In the PNG generator, the signature is

public function getBarcode($barcode, $type, int $widthFactor = 2, int $height = 30, array $foregroundColor = [0, 0, 0])

But in all the others, it's a string

public function getBarcode($barcode, $type, int $widthFactor = 2, int $height = 30, string $foregroundColor = 'black')

If they were the same, we could create a BarcodeGeneratorInterface, and each generator could implement it. I think that'd be better than simply extending the abstract class.

What do you think about creating a version 3 uses an interface and requires that $foregroundColor is a string in all the generators?

[tacman]

On a related note, the getBarcode signature is inconsistent in BarcodeGeneratorDynamicHTML

public function getBarcode($barcode, $type, string $foregroundColor = 'black')

[casperbakker]

What do you think about creating a version 3 uses an interface and requires that $foregroundColor is a string in all the generators?

Yes, that could work. Lots of these issues are legacy from 10+ years of this code' existence. Bit by bit we modernise it, which is nice.

For version 3 I am also thinking about pulling the BarcodeTypes and Generators further apart, where you can inject the results of a BarcodeType into a Generator (but with different names). That way it is easier to build or tweak your own generator. A better interface could also help with that.

[tacman]

So does this mean no version 3?

[casperbakker]

So does this mean no version 3?

No, even better: it means version 3 is finally here! Checkout the release

[tacman]

Oh, congrats! This was closed as "not planned" -- is it no longer relevant?

I need to update my Symfony bundle that is a simple wrapper about your great library: https://github.com/survos/BarcodeBundle

I remember trying to inject an interface when I ran into this problem. So is there a common interface now, where getBarcode() is consistent and can be added to an interface?

What else do I need to know about to update the bundle?

[casperbakker]

Thanks!

The best way to learn about v3 is to read the readme. I explained everything in there, also about upgrading from v2. But it is backwards compatible, as the old generators are now helpers to the new classes.

It still seems hard to make a general interface for the renderers, as some support providing the width and some a width factor. But if you see a nice way to implement it, please let me know.

[tacman]

If all the renderers implemented the same interface, then it's really just a matter of initializing the service. You could even configure which renderer you wanted globally and inject it to a service (like twig), with no knowledge of which one is is.

for the renderers that don't support a particular feature, you could simply not implement it (or optionally though an exception that the application could handle).

But certainly the method calls themselves shouldn't change based on the renderer.

So

class myRenderer implement BarcodeRendererInterface
{
}

and normalize all the calls. You could have a method like supportsWidth() and supportsWidthFactor(), but every class would need to implement it, so that the caller wouldn't know anything at all about png or svg or jpg, simply what the service can do.

[casperbakker]

Thanks. I understand how interfaces work, just not in this specific case. It sounds like it is only better on paper, but the situation does not call for this setup. Just interfaces because interfaces, does not help in practice.

For example, you cannot use the HTML barcodes the same way you can use the JPG/PNG images and different still from SVG's. So in usage you can also not just switch them. You always have to have different code before and after the rendering of the barcode, so conforming to an interface does not help. They are not interchangeable. It looks like they do the same thing, but not really.

for the renderers that don't support a particular feature, you could simply not implement it (or optionally though an exception that the application could handle).

For me, this is a big code smell. If it works differently, it should be different interfaces. The interfaces should define what to expect, not throwing exceptions or different helpers to know what will work. That is what the interface is for.

If it helps, I could make a different interface for all the renderers, if that makes building a different renderer better. But then it will be something like an ImageRendererInterface and a VectorRendererInterface. Would that help? If it is not really helpful, I would rather not add them.

Using dependency injection is also not needed I think. What different service would you inject?

[casperbakker]

@tacman I have proof-of-concept of the implementation of a RendererInterface: #206

Would something like that work?

I don't like that we make it simple to give a width that does not match a round factor in PNG/JPG, as it will make the barcode not compliant. But maybe that is a trade-off worth making.

[tacman]

Thanks! I wrote an article about creating a Symfony bundle, using this library as an example:

https://medium.com/@tacman1123/easily-share-your-twig-extensions-with-symfony-6-1-ff633bf7d9db

My idea for the bundle was that DX is this

• composer require survos/barcode-bundle
• configure survos_barcode.yaml with your default, including your preferred renderer
• in twig, render the barcode with one of these:

{% set id = 12345 %}
{{ id|barcode }}
{{ id|barcode(width='400px') }}
{{ barcode(id, {width: '400px'}) }}

e.g. https://barcode-demo.survos.com/

The underlying php for the twig is

    public function barcode(string $value, ?int $widthFactor = null, ?int $height = null, ?string $foregroundColor = null, string $type = BarcodeGenerator::TYPE_CODE_128, string $generatorClass = BarcodeGeneratorSVG::class ): string
    {
        if (!class_exists($generatorClass)) {
            $generatorClass = $this->barcodeService->getGeneratorClass($generatorClass);
        }
        /** @var BarcodeGeneratorSVG|BarcodeGeneratorPNG $generator */
        $generator = new $generatorClass();
        $barcodeData = $generator->getBarcode(
            barcode: $value,
            type: $type,
            widthFactor: $widthFactor ?? $this->widthFactor,
            height: $height ?? $this->height,
            // hack for array / string issue https://github.com/picqer/php-barcode-generator/issues/172
            foregroundColor: $this->barcodeService->getImageFormat($generatorClass) ? [0,0,0] : $foregroundColor ?? $this->foregroundColor
        );
        if ($this->barcodeService->getImageFormat($generatorClass)) {
            $barcodeData = base64_encode($barcodeData);
        }
        return $barcodeData;
    }

The underlying code to interact with the generators would benefit from an interface.

    public function getGeneratorTypes(): array
    {
        return (new \ReflectionClass(BarcodeGenerator::class))->getConstants();
    }
    public function getGeneratorClasses(): array
    {
        // we _could_ use classfinder to get all the classes that implement BarcodeGeneratorInterface
        return array_reduce([
            BarcodeGeneratorSVG::class,
            BarcodeGeneratorHTML::class,
//            BarcodeGeneratorDynamicHTML::class,
            BarcodeGeneratorPNG::class,
            BarcodeGeneratorJPG::class
        ], function ($carry, $className) {
            $carry[(new \ReflectionClass($className))->getShortName()] = $className;
            return $carry;
        }, []);
    }


    public function getImageFormat(string $generatorClass): ?string
    {
        return match ($generatorClass) {
            BarcodeGeneratorJPG::class => 'image/jpeg',
            BarcodeGeneratorPNG::class => 'image/png',
            default => null
        };
    }

So this is the motivation behind the interfaces, but I hear what you're saying about code smell. I know little about the internals, and in my own application this works fine. If I update the article, I'd probably add some new features like autowiring and skip listing the generators.