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

[Proposal] QRCoder2 #321

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[Proposal] QRCoder2 #321

wants to merge 1 commit into from

Conversation

natehitze
Copy link

Proposal for #242. This is just an example -- I didn't move everything that could be moved. For the most part I left the code as-is after moving it. I targeted .net 6 without putting much thought into it. I'm sure we could target a bit lower if desired. My goal was to keep dependencies out of QRCoder2.

Payloads

I pulled payloads out of PayloadGenerator into their own classes. Other libraries can expose new Payloads by referencing QRCoder2 and extending PayloadBase.

QR Code data

QRCodeGenerator translates a PayloadBase to QRCodeData. I didn't change anything here, except moving ECCLevel and EciMode to the Payloads namespace.

Renderers

I added a renderers namespace. I put a couple of the "easy" QR codes that had no dependencies there: PngByteQRCode and BitmapByteQRCode. I think the *QRCode classes should then be renamed *QRCodeRenderer. As an example, I added a library for QRCoder2.Renderers.ImageSharp which has a dependency on ImageSharp (but does nothing at the moment). Other library authors could provide their own renderers using only QRCoder2 as a dependency.

@codeputer
Copy link

Looking for a QR Code generator, and I'm too using .NET6 - the sample code here is not working with the Nuget package that is supplied https://github.com/codebude/QRCoder version 1.4.3... so I'm thinking it not ready for .net6 ? Just started to look into the source and see if I can help.. but thought I would drop this comment if you wanted to work on it together

@daveapsgithub
Copy link

Happy to help out too. A have a BIG project and it has all upgraded to .net 6 quite successfully, except for this.

@itarizin
Copy link

Happy to help too.
I love this project and I because I wanted to use it on my .NET 6 project, I've written the SVG and PNG generator classes locally, as new generators.
I'm using ImageSharp and ImageSharp.Drawing.
This allows me to produce not only PNG but many other formats.

The only downside is that I've ImageSharp dependencies. It's not a problem for me because the generators are within my project, and not in this library.

Would it become acceptable to add dependencies to this library?
PS: Happy to share the classes if you need them.

@sebafelis
Copy link

I also wont to use this project in my own .NET 6 project, howewer now this is not cross-platform solution. Do you plan to make this cross-platform and when?
PS: I think using ImageSharp is grate idea becouse it's meets these requirements.

@csturm83
Copy link
Contributor

The only downside is that I've ImageSharp dependencies. It's not a problem for me because the generators are within my project, and not in this library.

@itarizin I think a clean approach would be to invert the dependency. Have something like an IQrCodeRenderer interface exposed publicly in the core lib. Implementation projects/libs would then reference the core lib, providing an implementation using any dependency they wish.

E.g. ImageSharpRenderer : IQrCodeRenderer or SystemDrawingRenderer : IQrCodeRenderer or InsertImplHereRenderer : IQrCodeRenderer.

This assumes a common renderer agnostic contract can be extrapolated. Something like IQrCodeRenderer<T> (where T is the rendered output type) might even provide greater flexibility.

Something similar for Payloads like IQrCodePayload might be useful as well?

Disclaimer: I'm just spit-balling here. I haven't actually looked at the code in a few years 😀.

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

Successfully merging this pull request may close these issues.

7 participants