[WIP/QRCoder2] How do we handle changes during the 2.0 development?

[codebude]

Note: This issue is part of the planning of version 2 of the QRCoder. The meta-issue can be found here. These comments have been copied into this issue here, with the comments marked as such. If comments on the topic of this issue already exist in the meta thread, they have been copied here, naming the authors.

Topic

Do we do a code freeze of the current 1.X version or do we allow the further development of the old version in parallel (with the disadvantage that we have to constantly update these developments again in 2.0)?

[codebude]

I think the answer depends on the quantity of breaking changes (and ease of developers to upgrade). Due to the popularity of the QRCoder project (27 million downloads and counting), I personally would attempt to release 2.0 with only removal of System.Drawing dependencies, as 1.4.3 and newer do not contain these missing classes anyway. Essentially, I would attempt to have version 2.0 be 100% backwards compatible for the net6.0 tfm. If that was the case, I think there is no reason to continue development on 1.x.

On the other hand, if a complete rewrite of the exposed API is desired, then I would maintain bugfixes for 1.x if/when requested by a contributor. It is quite easy to set up GitHub to allow for this. At GraphQL.NET we can easily release bugfixes for old versions if needed, although we very rarely receive a request to do so; typically users need help upgrading if anything. Having excellent migration documentation really helps in these cases.

^{Originally posted by @Shane32 here.}

[codebude]

On the other hand, if a complete rewrite of the exposed API is desired, then I would maintain bugfixes for 1.x if/when requested by a contributor. It is quite easy to set up GitHub to allow for this. At GraphQL.NET we can easily release bugfixes for old versions if needed, although we very rarely receive a request to do so; typically users need help upgrading if anything. Having excellent migration documentation really helps in these cases.

My vote would be to clean up the public API and better organize the structure of the main project. Maybe have separate namespaces for the core logic, for payloads and for renderers at least (for better discoverability).

At present there are at least a few ways to create a QR Code. There are QrCodeGenerator.CreateQrCode instance methods (1) which delegate to the QrCodeGenerator.GenerateQrCode static method (2), and then there are also the GetQrCode static helper methods (3). Maybe just pick one preferred way moving forward?

That said, I had also been experimenting with a more fluent-like extension method approach:

  public static TRenderer RenderAs<TRenderer>(this QRCodeData data) where TRenderer : AbstractQRCode, new()
  {
      TRenderer renderer = new TRenderer();
      renderer.SetQRCodeData(data);
      return renderer;
  }

Usage:

string graphic = QRCodeGenerator.GenerateQrCode(textBoxQRCode.Text, eccLevel)
    .RenderAs<AsciiQRCode>()
    .GetGraphic(20);

There would be some factoring out of IDisposable needed to make it work. Not sure why the MemoryStream in PngByteQRCode needs to be a class variable. Seems as though the MemoryStream lifetime could be managed in the GetGraphics methods (plumbing the Stream through the helper methods). I also wasn't sure of the value of Dispose functionality in QRCodeData? Maybe @codebude could clarify if it could be removed.

Edit: Disregard the bit about the MemoryStream implementation in PngByteQRCode. I was looking at the PngBuilder IDisposable implementation by mistake. All *QrCode implementations implement IDisposable by way of AbstractQrCode's Dispose method, which simply disposes the QRCodeData.

Here is QRCodeData.Dispose (not actually releasing unmanaged resources, just setting to default values):

public void Dispose()
{
    this.ModuleMatrix = null;
    this.Version = 0;

}

Other random thoughts:

• Rename GetGraphics to Render ?
• Maybe leverage some abstraction like RenderOptions or RenderSettings to clean up some of the parameter lists that have gotten a bit unwieldy.
• Maybe add an abstract method or two to AbstractQRCode to enforce a minimal contract for all subclasses. E.g. Render(RenderOptions options)
• Rename various other classes and methods for clarity and consistency

^{Originally posted by @csturm83 here.}