Skip to content

JobExecution Visitor does not allow for side-effectful impls #392

@cwbriones

Description

@cwbriones

See JobExecution$Visitor, and the test case where this happens.

Scenario: You want to visit JobExecution<T> instances and take some action depending on the variant, but you don't necessarily return a value. You try to specify your visitor as Visitor<T, Void>.

Outcome: Because there's no way to return Void from your visit methods (intentionally) then you try to return null. Spotbugs yells at you that you're returning null from a non-nullable method.

In the test case linked above I worked around this by just using Visitor<T, Integer>, but that doesn't give the same implication of a consumer-like class like what you get when using Void as your return type.

I think we can either

  • Mark Visitor methods as explicitly @Nullable by default and require implementations to add it themselves.
    • NOTE: You must do this on the inferface. Adding @Nullable to the implementations is not type-safe due to nullability variance rules.
  • Add a second interface that is specifically for side-effectful visitors (like Consumer)

I'm more in favor of the first since it doesn't add even more classes to this and this isn't really a big issue at the moment.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions