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

Consider providing a const constructor for Version #97

Open
dcharkes opened this issue Jan 10, 2024 · 5 comments
Open

Consider providing a const constructor for Version #97

dcharkes opened this issue Jan 10, 2024 · 5 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@dcharkes
Copy link

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)

@sigurdm sigurdm added the type-enhancement A request for a change that isn't a bug label May 2, 2024
@sigurdm
Copy link
Contributor

sigurdm commented May 2, 2024

What's the use case?

@dcharkes
Copy link
Author

dcharkes commented May 2, 2024

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.

@sigurdm
Copy link
Contributor

sigurdm commented May 2, 2024

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('.')}';
}

@dcharkes
Copy link
Author

dcharkes commented May 2, 2024

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

@sigurdm
Copy link
Contributor

sigurdm commented May 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants