Add back named constructor arguments (or include as an option) about protobuf.dart HOT 12 CLOSED

Thanks for the feedback. We're adding a flag to generate old style code with constructor arguments in #855, so no need to add more use cases/examples here.

from protobuf.dart.

I think the breaking change (v20 -> v21) is making the regression of the code readability.
Quoting the upper response @agreaves made,
Nested object to be almost unreadable if the params are getting more and more.

Getters and setters are useful when the class parameters are changing.
I understand the grpc has default values and we are re-assigning them.
But virtually, We are making a new object to use the grpc api.
I don`t know what is right to use.
But the code readability side, i think the named-constructor is way better solution.

final top = Top()
    ..a = (Middle()
      ..a = (Bottom()..a = 1..b = 2..c = 3)
      ..b = (Bottom()..a = 1..b = 2..c = 3)
      ..c = (Bottom()..a = 1..b = 2..c = 3))
    ..b = (Middle()
      ..a = (Bottom()..a = 1..b = 2..c = 3)
      ..b = (Bottom()..a = 1..b = 2..c = 3)
      ..c = (Bottom()..a = 1..b = 2..c = 3))
    ..c = (Middle()
      ..a = (Bottom()..a = 1..b = 2..c = 3)
      ..b = (Bottom()..a = 1..b = 2..c = 3)
      ..c = (Bottom()..a = 1..b = 2..c = 3));

final top = Top(
    Middle(
      Bottom(1, 2, 3),
      Bottom(1, 2, 3),
      Bottom(1, 2, 3),
    ),
    Middle(
      Bottom(1, 2, 3),
      Bottom(1, 2, 3),
      Bottom(1, 2, 3),
    ),
    Middle(
      Bottom(1, 2, 3),
      Bottom(1, 2, 3),
      Bottom(1, 2, 3),
    ),
);

from protobuf.dart.

In addition, one other extremely important function of constructor arguments is the ability to pass in null values (based on some condition). To use the example I mentioned in the original, you could previously do:

Person(
  name: givenName,
  middleName: hasMiddleName ? middleName : null,
  favoriteDinosaur: isNoFun ? null : dinoSelection,
)

This you could accomplish in one encapsulated statement, and means it can be used in many contexts (e.g. passing it in as an argument). Now, the equivalent would be:

final person = Person()..name = givenName;
if (hasMiddleName) {
  person.middleName = middleName;
}
if (!isNoFun) {
  person.favoriteDinosaur = dinoSelection;
}

which is both far less readable and far less convenient.

from protobuf.dart.

Besides what has already been mentioned above, this change is particularly harmful in the case of nested message.

Given:

class Top {
  Top({this.a, this.b, this.c});

  Middle? a;
  Middle? b;
  Middle? c;
}

class Middle {
  Middle({this.a, this.b, this.c});

  Bottom? a;
  Bottom? b;
  Bottom? c;
}

class Bottom {
  Bottom({this.a, this.b, this.c});

  int? a;
  int? b;
  int? c;
}

The following:

final top = Top(
    a: Middle(
      a: Bottom(a: 1, b: 2, c: 3),
      b: Bottom(a: 1, b: 2, c: 3),
      c: Bottom(a: 1, b: 2, c: 3),
    ),
    b: Middle(
      a: Bottom(a: 1, b: 2, c: 3),
      b: Bottom(a: 1, b: 2, c: 3),
      c: Bottom(a: 1, b: 2, c: 3),
    ),
    c: Middle(
      a: Bottom(a: 1, b: 2, c: 3),
      b: Bottom(a: 1, b: 2, c: 3),
      c: Bottom(a: 1, b: 2, c: 3),
    ),
  );

will become:

  final top = Top()
    ..a = (Middle()..a = (Bottom()..a = 1..b = 2..c = 3)..b = (Bottom()..a = 1..b = 2..c = 3)..c = (Bottom()..a = 1..b = 2..c = 3))
    ..b = (Middle()..a = (Bottom()..a = 1..b = 2..c = 3)..b = (Bottom()..a = 1..b = 2..c = 3)..c = (Bottom()..a = 1..b = 2..c = 3))
    ..c = (Middle()..a = (Bottom()..a = 1..b = 2..c = 3)..b = (Bottom()..a = 1..b = 2..c = 3)..c = (Bottom()..a = 1..b = 2..c = 3));

This forces us to choose between two equally poor choices, continue using method chaining at the cost of readability, or refactor the code to use local variables which makes the code significantly more verbose.

from protobuf.dart.

oh that is why it stopped working
any advice how to dart pub global activate protoc_plugin with last working version?

i found it

dart pub global deactivate protoc_plugin
dart pub global activate protoc_plugin 20.0.1

from protobuf.dart.

Assuming that the command above is correct (I trust it, just haven't checked) and knowing that the latest version of dart protoc is 20.0.2, our friends here introduced large backwards API incompatibility when increasing the version only by 0.0.1.

API breaking changes like this has to be planned long in advance with deprecation and not simple removal with a bump of a minor library version.

from protobuf.dart.

Just want to take a moment to thank the team here, especially @osa1 for that PR. I'm sure even frustrated users understood this was just a change made too quickly, and I'm really happy that it was resolved. In fact, my team hadn't even updated the package since the removal, and now we won't even feel the issue at all.

from protobuf.dart.

I'd like to express the same opinion, this update seems really half-baked and breaks codebases in a spectacular way.

It hurts the code quality the most when using optional fields, with a perfect example from @agreaves .

Please reconsider this decision, as we'll probably have to stick with version <21, which is not ideal.

from protobuf.dart.

To add to this, my team uses Protobuf to sync data between C++, Python, and Dart. We use protobuf messages alongside regular classes in Dart and draw no distinction between the two. When we want to sync some data to the other platforms, we just transfer the declaration from Dart to Protobuf.

This change means we'll have to track which classes are written in Dart and which are generated by Protobuf in order to use them correctly. It also means that changing a class to a message is a much more involved process, where we have to change every time it is constructed. We'd much prefer the current way, with the constructor arguments, even if it results in a slightly larger executable. Keep in mind that flutter already produces a fair bit of bloat, which we are more than okay with.

from protobuf.dart.

oh that is why it stopped working any advice how to dart pub global activate protoc_plugin with last working version?

i found it

dart pub global deactivate protoc_plugin
dart pub global activate protoc_plugin 20.0.1

Thanks for the helpful command. It's ridiculous that they've decided to break existing code just like that without even trying to make it a parameter or something.

from protobuf.dart.

our friends here introduced large backwards API incompatibility when increasing the version only by 0.0.1

This change was introduced with a major version bump from protoc_plugin-20 to 21.

from protobuf.dart.

How do you fill repeated fields? I don't see a setter in the generated code, and my protoc does not have the new option.
Never mind
#8
explains it.

from protobuf.dart.