sviperll / adt4j

adt4j - Algebraic Data Types for Java
BSD 3-Clause "New" or "Revised" License
143 stars 8 forks source link

Allow optional generation of Tag enum #30

Open sviperll opened 8 years ago

sviperll commented 8 years ago

Add @GenerateTags annotation to generate enum with tag-values.

Following code

    @GenerateTags(enumName = "OptionalTag", methodName = "tag")
    @GenerateValueClassForVisitor(className = "Optional")
    @Visitor(resultVariableName="R")
    interface OptionalVisitor<T, R> {
        R present(@Nonnull T value);
        R missing();
    }

should generate

    enum OptionalTag {
        PRESENT, MISSING
    }
    class Optional<T> {
        OptionalTag tag() {
            // ...
        }
        <R> accept(OptionalVisitor<T, R> visitor) {
            // ...
        }
    }
rouzwawi commented 7 years ago

Are there any plans on implementing this?

We have many visitor implementations that only care about a few tags and just return a default value for all other tags. When there's many tags (like in an ADT representing some Event), we get very large visitors. Having a generated tag enum would allow us to use regular switch statements with a default case.

jbgi commented 7 years ago

Another solution would be generating a VisitorAdapter which have a single abstract method R defaultValue() to which every other methods delegate to (as default implementation).

sviperll commented 7 years ago

I think VisitorAdapter solution is a better way than switch-statement. Another feature that I plan to implement that is beneficial to Event-like scenario is "branchy-visitors" as described in #31 . This should allow splitting of large visitors into several smaller visitors.

Currently I plan to move away from jcodemodel to homegrown library for underlying implementation. Currently it's incomplete, but I hope that it will allow faster adt4j development after switch. I can describe motivation, design and implementation somewhere if you are interested.

rouzwawi commented 7 years ago

@sviperll I'm not sure how the "branchy-visitors" would help in my case, as it seems to be a structure inherent to the type. We have many different groupings of the tags that are applied in different visitors. i.e. given:

public interface EventVisitor<R> {
  R timeTrigger();
  R triggerExecution();
  R created(String executionId);
  R started();
  R terminate(int exitCode);
  // ...
}

And in some visitors, we only discriminate say terminate(int) and started() and want a default value for everything else. Then some other grouping in another visitor. As I understood the branchy visitor, these groupings would be inherent to the ADT and no the use-sites.

sviperll commented 7 years ago

@rouzwawi Yes, you are right about branchy-visitors. For use-site groupings I think you can develop some set of adapter-visitors that will cover your use-cases.

rouzwawi commented 7 years ago

@sviperll @jbgi yes, a VisitorAdapter using default implementations delegating to a defaultValue() would do the job for my use case.

However, I'm slightly more for the tag enum as it leads to easier-to-read code. But there's of course a tradeoff.

I see the main benefit of the visitor to be the compile time validation of exhaustiveness. In this use case, I'm explicitly opting out of the exhaustiveness of the visitor by using a default value for all the uncovered cases. Then on the other hand, a Java switch statement is not an expression, so there's room for errors in missing to declare the default case, or missing to assign the variable (could be solved by declaring it final though).

Overall, no strong opinion about either solution as they both solve my use case :)