2

Thinking about misusing sealed classes, let's look at the following design.

There are two modules:

  • parser (Kotlin) - is responsible for making instances from String
  • processor (Java) - pumps over raw incoming data into a strongly typed storage (i.e. relational tables)
  1. String comes from external source to processor
  2. processor delegates its recognition to parser
  3. parser makes instances of different types [Banana, Nail, Shoe] based on some rules X
  4. processor persists each instance into an appropriate table based on some rules Y

Is it appropriate to use sealed classes here like this in parser and after that in processor make decisions base on concrete type of each instance?

// parser module exposes Item and its subclasses

sealed interface Item {
    class Banana(/*state 1*/) : Item
    class Nail(/*state 2*/) : Item
    class Shoe(/*state 3*/) : Item
}

fun parse(value: String, rule: ParseRule): Item {
    return when (true) {
        rule.canParseBanana(value) -> rule.makeBananaFrom(value)
        rule.canParseNail(value) -> rule.makeNailFrom(value)
        rule.canParseShoe(value) -> rule.makeShoeFrom(value)
        else -> throw RuntimeException("cannot parse")
    }
}    

// processor module makes decisions based on class 

void process(String value){
  Item item = parser.parse(value);

  if (item instance of Item.Banana){
    persistBanana((Item.Banana) item)
  } else if ( ... )
    // etc         
  } else {
     throw new RuntimeException("Unknown subclass of Item : " + item.getClass())
  }
}

I see that something is wrong in this approach, because growing number of Item's subclasses could lead to a design catastrophe, but cannot figure out whether is there a "canonical" use case of sealed classes sufficiently different from this one.

What is the limit of sealed classes applicability, when system designer should prefer something "less typed" like the following:

class Item{
  Object marker; // String or Enum
  Map<String, Object> attributes;
}

// basically it is the same, but without dancing with types
void process(String value){
    Item item = parser.parse(value);

    if ("BANANA".equals(item.marker)){
      persistBanana(item.attributes)
    } else if (...){
      // etc
    }
}
diziaq
  • 6,881
  • 16
  • 54
  • 96
  • 1
    I believe questions you asked apply to both Kotlin and Java. You should always try to stick to a proper OOP, so overloading/implementing methods instead of using when/switch/if chains. Then it doesn't matter how many subclasses there are. Sometimes, it is better to use enums/sealed interfaces, but usually it happens when there is a finite set of possible elements and they won't change too much in the future. And if it makes logical sense that `when` code knows all possible subtypes, because often it really shouldn't. – broot Jul 10 '21 at 10:36
  • 1
    It is hard for me to suggest a specific solution to your case. It could make sense to make items persist themselves, e.g. add function to `Item`: `fun persist()`. On the other hand database layer by definition knows all entities, so it sometimes could make sense to create such if chains. I definitely discourage using strings as in your last example. Sealed interfaces or enums are much better for such cases. – broot Jul 10 '21 at 10:39
  • Removing the `sealed` keyword from this example doesn't change anything, which seems to answer the question of whether it is appropriate here. It isn't adding any value. The real question here doesn't appear to be about `sealed` at all; but rather, the most fundamental question in OOP: how to model an abstraction with different concretions. – jaco0646 Jul 10 '21 at 15:58
  • @jaco0646 You're right. I guess you expressed my thoughts better than me. In fact the question was about the limitations of sealed classes. I intentionally misused them (as their removing does not affect the design). The problem is I do not see when it is appropriate (or maybe inevitable) to apply sealed classes. And looking at the answers I see that I am not alone in this confusion. – diziaq Jul 12 '21 at 13:26
  • Have you read some of the popular threads about sealed classes, such as [Why seal a class?](https://stackoverflow.com/questions/268251/why-seal-a-class) and [When and why would you seal a class?](https://stackoverflow.com/questions/7777611/when-and-why-would-you-seal-a-class) Many languages have this feature, so the question is more or less language agnostic. – jaco0646 Jul 12 '21 at 14:09

2 Answers2

1

You can use the visitor pattern to provide a when..is style approach to Java.

// Kotlin
abstract class ItemVisitor<OUT> {
    operator fun invoke(item: Item) = when (item) {
        is Banana -> visitBanana(item)
        is Shoe -> visitShoe(item)
        is Nail -> visitNail(item)
    }
    abstract fun visitBanana(item: Banana): OUT
    abstract fun visitShoe(item: Shoe): OUT
    abstract fun visitNail(item: Nail): OUT
}

Because Item is sealed you don't need the else case in that when and then Java code can create a visitor instead of doing its own instanceof checks with an else, and if you add a variant, you add a method and get reminded that your Java implementations of the visitor need the new method.

Mike Samuel
  • 118,113
  • 30
  • 216
  • 245
  • 1
    The Visitor Pattern is used to avoid type checking, not to incorporate it. The `invoke` method contradicts the pattern. – jaco0646 Jul 10 '21 at 15:30
  • @jaco0646 This consolidates type checking in one place in Kotlin where the compiler recognizes that all cases are covered so you can avoid doing it in Java where the compiler doesn't. – Mike Samuel Jul 13 '21 at 15:28
  • Agreed, this may be idiomatic Kotlin; but it is the opposite of the Visitor pattern. Visitor avoids type checking altogether. (But on a related note, Java is implementing a similar type checking feature with [Pattern Matching for switch](https://openjdk.java.net/jeps/406).) – jaco0646 Jul 13 '21 at 16:27
1

If you choose to use sealed classes you take it upon yourself to extend the hierarchy whenever a new subtype is introduced. Given that you have a subtype specific rules and persistence I am not sure how much you can do to avoid this.

Having said that, it's a shame to do instanceOf after you already determined the item type. The code below is more or less visitor pattern(this is disputed, see comments). I assume the separation to Java and Kotlin is required.

// Kotlin
sealed interface Item {
    class Banana(/*state 1*/) : Item
    class Nail(/*state 2*/) : Item
    class Shoe(/*state 3*/) : Item
}

class Parser(val persistor: Persistor, val rule: ParseRule) {
    fun parse(value: String): Item =
        when (true) {
            rule.canParseBanana(value) -> rule.makeBananaFrom(value).also { persistor.persist(it) }
            rule.canParseNail(value) -> rule.makeNailFrom(value).also { persistor.persist(it) }
            rule.canParseShoe(value) -> rule.makeShoeFrom(value).also { persistor.persist(it) }
            else -> throw RuntimeException("cannot parse: $value")
        }
}

class ParseRule {
    fun canParseBanana(value: String): Boolean = ...
    fun makeBananaFrom(value: String): Item.Banana = ...
    ...
}
// Java
public class Persistor {
    void persist(Item.Banana item) {
        System.out.println("persisting " + item);
    }
    void persist(Item.Shoe item) {
        System.out.println("persisting " + item);
    }
    void persist(Item.Nail item) {
        System.out.println("persisting " + item);
    }
}


public class Processor {
    private final Parser parser;

    public Processor(Parser parser) {
        this.parser = parser;
    }

    void process(String value) {
        parser.parse(value);
    }
    
    public static void main(String[] args) {
        Parser parser = new Parser(new Persistor(), new ParseRule());
        Processor p = new Processor(parser);
        p.process("banana");
        p.process("nail");
        p.process("zap");
    }
}


David Soroko
  • 8,521
  • 2
  • 39
  • 51
  • There is no Visitor Pattern in this solution, which is fine because this solution doesn't need it. – jaco0646 Jul 10 '21 at 15:42
  • You say this because you miss the double dispatch? I don't really feel strongly about this but `persistor.persist(it)` has that kind of quality. – David Soroko Jul 10 '21 at 15:54
  • There is no double dispatch in calling the `persist` method, because there is no type ambiguity. There is no need for double dispatch because the `rule` returns concrete types directly. To phrase it another way, calling an overloaded method does not automatically mimic double dispatch the way the Visitor Pattern does. Visitor retrieves "lost" type information. In this solution, nothing is lost. This is actually a better solution because by retaining the type information generated from the parser, the complexity of a Visitor is unnecessary. – jaco0646 Jul 10 '21 at 16:11
  • I believe that double dispatch is an implementation detail and not a prerequisite for visitor at least according to GoF. Anyway, I'll record your objection. – David Soroko Jul 10 '21 at 16:29
  • To be 100% clear: there is no actual double dispatch in the Visitor Pattern. Double dispatch is a language feature. Visitor only _mimics_ that feature, in languages that do not directly support it. In languages that do support it, there is no reason to use the pattern. This solution has no need to even mimic that feature, and thus no need to mention Visitor at all. – jaco0646 Jul 10 '21 at 16:42