pub_semver icon indicating copy to clipboard operation
pub_semver copied to clipboard

Consider providing a const constructor for `Version`

Open dcharkes opened this issue 1 year ago • 5 comments

I'm not entirely sure what to do with _text though.

Use case: I'd like to specify the version of things as consts.

(low prio)

dcharkes avatar Jan 10 '24 16:01 dcharkes

What's the use case?

sigurdm avatar May 02 '24 09:05 sigurdm

class SomeClass {
  static final Version latestVersion = Version(1, 2, 0);
}

Feels more natural to write const for something like this. If it would have been an int or string it would have been mandated const by a lint. And Version really feels like a data class to me that has all info when being constructed.

Also, if Version is a field of a class that could otherwise have a const constructor.

dcharkes avatar May 02 '24 09:05 dcharkes

I've personally never really gave much for constness.

But I guess there are cases where you need it (eg. for default argument values).

Looking at the code, it seems to be the fact that preRelease and build are represented as a list of elements that makes this hard. We can concatenate into _text at const time.

Eg. this simplified thing works

class Version {
  final String text;
  final int major;
  final int minor;
  final int patch;

  const Version(this.major, this.minor, this.patch)
      : text = '$major.$minor.$patch';
}

But this does not:

class Version {
  final String text;
  final int major;
  final int minor;
  final int patch;
  final List<String> pre;

  const Version(this.major, this.minor, this.patch, this.pre)
      : text = '$major.$minor.$patch-${pre.join('.')}';
}

sigurdm avatar May 02 '24 10:05 sigurdm

Yes, we'd have to make _text nullable, and recompute on every toString() invocation if it's null.

dcharkes avatar May 02 '24 10:05 dcharkes

Yeah - probably not too bad.

We could also consider throwing away the original text representation entirely. Not sure why we want to preserve non-canonical data here - it has bitten us several times on pub.dev.

sigurdm avatar May 02 '24 11:05 sigurdm