Good pattern for easily instantiating messages that contain a oneof

[Spencer Judge]

Hello,

Was wondering if there is an easier way to implement this pattern. Right now, if I have some definitions like this:

message DefHolder {
  oneof definition {
    AcceptNodeDef an = 100;
    RestartGateNodeDef rn = 101;
    TCNodeDef tn = 102;
    WaitForParentNodeDef wn = 103;
    CommitNodeDef cn = 104;
    PromoteArtifactsNodeDef pn = 105;
    EndNodeDef en = 106;
    //    NoOpNodeDef nn = 107;
  }
}
// NODE DEFINITIONS ===============================================================================

message AcceptNodeDef {
  option (scalapb.message).extends = "DAGNodeDefinition";
  int32 id = 1;
  string name = 2;
  NodeStage stage = 3;
  string description = 4;
  int32 maxRetries = 5;
}

message RestartGateNodeDef {
  option (scalapb.message).extends = "DAGNodeDefinition";
  int32 id = 1;
  string name = 2;
  NodeStage stage = 3;
  string description = 4;
  int32 maxRetries = 5;
  uint64 waitCutoff = 6 [(scalapb.field).type = "scala.concurrent.duration.FiniteDuration"];
}

etc, etc

It's quite annoying to instantiate a DefHolder. Essentially, I end up with code like this:

new DefHolder(dnd match {
                case a: AcceptNodeDef => An(a)
                case r: RestartGateNodeDef => Rn(r)
                case t: TCNodeDef => Tn(t)
                case w: WaitForParentNodeDef => Wn(w)
                case c: CommitNodeDef => Cn(c)
                case p: PromoteArtifactsNodeDef => Pn(p)
                case e: EndNodeDef => En(e)
              })

Which is pretty un-ergonomic. Is there any easier way to instantiate these types of messages? I know that it would probably be easier to make the DefHolder simply directly be DAGNodeDefinition, in this example, but I am converting a system with a bunch of existing code to using protobufs, and being able to re-usse the existing traits is very helpful in reducing the amount of code I need to modify.

I would appreciate any suggestions, otherwise, I would be happy to open an issue about making this improvement.

Spencer

[Nadav Samet]

Similar syntax that is also available is through the update function:

DefHolder().update(_.an := a)

or

DefHolder().withAn(a)

Both help you bypass the constructors (An, Rn, ...) and assign directly.

Are you looking for a way to avoid the pattern matching? We can probably code generate it, but only when the types inside the one-of are unique, so it's not ambiguous. 

--
You received this message because you are subscribed to the Google Groups "ScalaPB" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalapb+unsubscribe@googlegroups.com.
To post to this group, send email to sca...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/scalapb/81573da6-4940-48c6-b07f-0cff2f38e399%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

-Nadav

[Spencer Judge]

Thanks, I did figure out that somewhat better syntax a bit after I sent this. That said, yes, the pattern matching is what I'm trying to avoid. Sounds like it would require a change to the codegen, then. I expect one way to do it would be a custom proto option that says "This trait's inheritors are the same as this oneof's options" or you could have an option that says "the inner sealed trait you generate for this oneof should inherit from this other trait" which would probably be pretty easy to implement but I think not quite as type safe.

In any case, thanks for the info. I'll stick with the manual way for now but this would be a neat feature since it's a pattern that's popped up a few times for me already.

Spencer

To unsubscribe from this group and stop receiving emails from it, send an email to scalapb+u...@googlegroups.com.

To post to this group, send email to sca...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/scalapb/81573da6-4940-48c6-b07f-0cff2f38e399%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

-Nadav

[Spencer Judge]

So I have run into this a few more times, and I think I have a proposal for a good solution to at least part what makes working with oneofs hard.

One of the main problem with the oneof fields, for me, is that there is no simple way to access the "value" field of the non-empty members. You essentially have to check that it's not empty, then figure out what its real type is and cast it to that simply so that you can then access the real value. A really simple addition here that would make things quite a bit simpler would be a "getValue" method on the generated trait which returns an Option[GeneratedMessage]. Even better, would be if during codegen all types in the oneof were assigned a trait like "Valued<oneofname>" so that the return type of "getValue" can be more specific. I'm not sure if that violates some codegen ordering rules, so maybe that's not doable, but it'd help.

To illustrate why this would be nice with an example:

message TaskHolder {
    bytes id = 1 [(scalapb.field).type = "java.util.UUID"];
    int32 pending_cl = 2;

    oneof task {
        TaskA a = 4;
        TaskB b = 5;
        TaskC c = 6;
        ... etc etc ...
    }
}

Now let's say TaskB and TaskC both inherit from some trait indicating they are cancellable tasks, for example. With the current codegen, if I want to check if the task inside a task holder is a cancellable task, I really don't have any way to do that besides a big matcher with an option for every task type. With my proposed changes, I could just do:

tholder match {
  case TaskHolder(_, _, t) if t.getValue.map(_.isInstanceOf[CancellableTask]) => true
  case _ => false
}

Still not the prettiest thing ever, but a lot less code than a big match with every task type.

Does that seem good? Maybe there's a better option? I'd be happy to make a PR with the change, although it might be quite some time before I can get around to it :).

Thanks!
Spencer

To unsubscribe from this group and stop receiving emails from it, send an email to scalapb+unsubscribe@googlegroups.com.

To post to this group, send email to sca...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/scalapb/81573da6-4940-48c6-b07f-0cff2f38e399%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

-Nadav

[Nadav Samet]

It would be fairly easy to add getValue though it would have to return `Any` since oneofs may contain ints, strings and so on, not just messages (feel free to create an issue in GitHub). The `Valued[OneofName]` wouldn't work because of this reason (also because the code for the one-of different possibilities may be generated at entirely on different project - the container is unknown at that time).

To view this discussion on the web visit https://groups.google.com/d/msgid/scalapb/c181fd8d-b55a-42fd-8d56-d6fb6c52677e%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

-Nadav

[Ólafur Páll Geirsson]

Hi! FWIW, I opened a ticket https://github.com/scalapb/ScalaPB/issues/455 proposing an alternative code generation scheme for "sealed oneof", a special case of oneofs that meet the criteria that all fields of the oneof are defined in the same proto file. 

Cheers,

To unsubscribe from this group and stop receiving emails from it, send an email to scalapb+u...@googlegroups.com.

To post to this group, send email to sca...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/scalapb/81573da6-4940-48c6-b07f-0cff2f38e399%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

-Nadav

--
You received this message because you are subscribed to the Google Groups "ScalaPB" group.

To unsubscribe from this group and stop receiving emails from it, send an email to scalapb+u...@googlegroups.com.

To post to this group, send email to sca...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/scalapb/c181fd8d-b55a-42fd-8d56-d6fb6c52677e%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

-Nadav