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

Why does Barcode.build() always return a singleton list? #228

Open
maresb opened this issue Apr 27, 2024 · 2 comments
Open

Why does Barcode.build() always return a singleton list? #228

maresb opened this issue Apr 27, 2024 · 2 comments
Labels
contributions welcome Further design discussions and PRs are welcome.

Comments

@maresb
Copy link
Contributor

maresb commented Apr 27, 2024

The return type of Barcode.build() is List[str]. But in all overrides of .build(), the return value is always a singleton. I'm trying to understand the source code, and I find this really confusing, so I was wondering if there's an explanation. Is it for historical / backwards compatibility reasons?

I would have expected the return type to be str, so then I want to understand what lists of different lengths are supposed to mean. Maybe it would be good to at least include an explanatory comment to reduce confusion? Thanks!

maresb added a commit to maresb/labelle that referenced this issue Apr 27, 2024
maresb added a commit to maresb/labelle that referenced this issue Apr 27, 2024
@WhyNotHugo
Copy link
Owner

Is it for historical / backwards compatibility reasons?

This is almost always the answer for anything unclear in this codebase. It is a fork of an older, unmaintained, library from over a decade ago, and it's been hard to clean up these quirks without breaking backwards compatible.

At this point, I'm okay with making breaking API changes as long as they're clear and well documented. New versions follow semver to indicate breaking changes.

The return type of Barcode.build() is List[str]. But in all overrides of .build(), the return value is always a singleton.

True, I've confirmed that they all return a list with a single string. We can definitely change this to simply return a string instead.

@WhyNotHugo WhyNotHugo added the contributions welcome Further design discussions and PRs are welcome. label Apr 28, 2024
maresb added a commit to maresb/labelle that referenced this issue Apr 28, 2024
@maresb
Copy link
Contributor Author

maresb commented Apr 29, 2024

Thanks a lot for the clear explanation. I feel for you on the quirky legacy codebase. All that callback nonsense is totally out of control. It'd be great to eventually have a clean functional interface, but I can see that it's a ton of work.

maresb added a commit to maresb/labelle that referenced this issue Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions welcome Further design discussions and PRs are welcome.
Projects
None yet
Development

No branches or pull requests

2 participants