Null Safety support about crclib.dart HOT 9 CLOSED

In case someone needs it, for now you can import null safety version in pubspec.yaml like this

dependencies:
  crclib:
    git:
      url: https://github.com/google/crclib.dart.git
      ref: nullsafety

from crclib.dart.

I have published 3.0.0 a few minutes ago. It should be live soon.

from crclib.dart.

Does https://github.com/google/crclib.dart/tree/nullsafety have the feature you want?

from crclib.dart.

The nullsafety branch seems perfect. Can you publish a new version on pub.dev @postmasters ?

from crclib.dart.

I've planned to wait a little bit longer to release that branch as a new version (maybe August / September so that the null safety concept has time to sink). For now, may I ask you to please specify Git tag/commit hash to use it in your pub.yaml?

from crclib.dart.

Hi @postmasters, I don't understand why you are suggesting to wait to release a null safe release when the whole ecosystem is making the move and this library looks pretty stable.

I feel it's bad practice tagging onto a commit hash (especially in a build which you need to be reproducible in the future) as it could go away at any time with a force-push in git whilst a published library will remain available.

Why not publish a 3.0.0+nullsafe.0 prerelease which has null safety if you're worried about API stability?

I had a quick look at your nullsafety branch and it looks pretty standard for a null-safe migration, I do have a couple of questions / changes.

1. Turn CrcValue into an abstract class with a factory constructor and separating out the int/BigInt implementations. This will tidy up the use of nulls in CrcValue.

abstract class CrcValue {
  factory CrcValue(dynamic value) {
    if (value is int) return CrcValueInt(value as int);
    if (value is BigInt) return CrcValueBigInt(value as BigInt);
    throw Exception("CrcValue must be an int or BigInt");
  }
}

It would be cool if IntCrcValue and BigIntCrcValue could be done using a generic CrcValueImpl but I don't think there is a common interface for the two off the top of my head.

2. In solveAugmentedMatrix - null is returned when "The pivot index is not in range, the system is inconsistent". Should this be an Exception?

3. flipWithValue - null is returned when allowedPositions is empty. Should this be checked prior to calling flipWithValue / flipWithData so the return value of flipWithValue becomes non-nullable and instead throws an Exception if an empty list is encountered? Making solveAugmentedMatrix non-nullable also gets rid of the null check question mark

With 2 & 3 I'm unsure if they are unexpected cases which are serious enough to throw exceptions as you currently let them fail through null.

Many thanks for a great lib!
James

from crclib.dart.

Thanks for the inputs, James! I'll go through them in order.

0. Why not publish nullsafety branch? This is pretty much a personal preference. I don't want to maintain two packages. So I'll wait for an arbitrary cut over date and will rebase main on the nullsafety branch. And that will go out as one single package versioned 3.0.0. I can commit to keep the tag nullsafety where it is before that cut off going forward, if that helps you pin the version.

And these side discussions (feel free to open new issues):

1. Turning CrcValue into an abstract class is a nice touch. PRs welcomed! On the other thand, I'm not happy with CrcValue. This shim class does not contribute to speed, nor space. I might later decide to up-cast int to BigInt and make it the result of a CRC calculation.
2. Should solveAugmentedMatrix throw an exception? I think not. It's perfectly valid to have a system of equations that does not have any solution.
3. Similarly, a null from flipWithValue signals no solution, rather than an exception (i.e. provided inputs were handled fine). Also, if the final value is the same as the initial value, then no flipping is required either. I thought it was handled but the placement in current code was wrong (it should have been done before the empty allowedPositions check).

Thanks and keep new issues coming!

from crclib.dart.

So null safety is now main stream.
Can we get this package published please.

from crclib.dart.

Ah right. So sorry for dropping the ball and thank you for the ping. I'll get to it ASAP.

from crclib.dart.