Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency in $foregroundColor (string and array) #172

Open
tacman opened this issue Oct 3, 2022 · 2 comments
Open

Inconsistency in $foregroundColor (string and array) #172

tacman opened this issue Oct 3, 2022 · 2 comments

Comments

@tacman
Copy link

tacman commented Oct 3, 2022

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
Copy link
Author

tacman commented Oct 13, 2022

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

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

@casperbakker
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants