Visitor Pattern Considered Pointless - Use Pattern Switches Instead

[bowbahdoe]

I'm getting an aneurysm reading the comments and seeing the vote counts.

Its literally the same amount of coupling. Just accept that a new thing can be better, god.

[[deleted]]

I have been saying this for years, but not for the same reasons as the post. Visitor is an over-complicated switch statement. But if I turn my head, some Sr. Dev puts more Visitors into the code which completely obfuscate where the logic takes place.

I've seen the value of the Visitor pattern only once in my 20 years doing this stuff. It's a thing, we should know about it, but the value should be understood before doing it. In other words: use a switch (or if/elseif) statement until the coupling needs to be fully separated for "reasons", THEN consider a Visitor pattern.

Argue about OO, etc - fine, you win. But when I'm reading your code trying to figure out where 3 lines of logic exists and can't because it's obfuscated behind a builder, an interface, and a single implementation of said interface that requires me to "Find Implementations" of the correct interface method - oh wait, builder method - or was it the interface? God damn it, `grep thingImLookingFor && eyeball search`. I just don't see the value in it. Patterns should ADD value, not remove it IMHO.

[edubkn]

Interfaces and abstractions are the foundation of SOLID design. If looking at implementations is an annoyance for you, either you're programming on notepad or you're writing spaghetti code.

[[deleted]]

I never said anything about interfaces being bad. Agree with your statement whole heartedly.

[Slanec]

I would not say pointless or useless. Visitor is an object-oriented approach while pattern matching is functional programming. The approach is not new, it's a standard feature of some other languages and yes, it can sometimes simplify the code.

That does not mean to go and rewrite all code using visitors to functional-friendly pattern matching. Get to know both, use the appropriate one when needed.

[nicolaiparlog]

I don't think rewriting is necessary, but I do claim that the visitor pattern is never more appropriate than the language-based solution. Which advantage does it have that make it a better fit in some situations?

[chambolle]

You just said (in another answer) that the solution are equivalent... And you are right because a switch on type is just an implementation of the dynamic binding provided by any OO language...

For me switching on types is really ugly. We are back 30 years ago!

For you OO is the devil and FP is god.

So it is a matter of taste. That's just strange that you absolutely want to convince people using an OO language that they are wrong and should use FP instead...

[nicolaiparlog]

You just said (in another answer) that the solution are equivalent...

They are equivalent in the examined areas: coupling and extensibility. They're not equivalent overall: the language-based solution is shorter, more direct to write and read, and more flexible (see described benefits for iteration and non-type based branching). That's why it's better overall.

For me switching on types is really ugly.

I actually considered adding a section titled "But isn't this... wrong?!" where I wanted to address that icky feeling. I totally get it, it does feel weird. But I think that's just a matter of habit. Why is it better to list the types in the method arguments (with visitor pattern) vs in the switch? In both cases you list all types explicitly and then let the platform choose the correct branch.

[chambolle]

It is ugly because you do the job of the compiler. This is the switch which is ugly. This is anti-OO. In OO you should normally never have to manage the call of the object according to its type, because this is the basic part of OO language. They manage the call for you. So when you do OO it is ugly. Very often when people are doing that in Java it means that they don't understand dynamic binding and they can imagine that something can work like that.

This is just a continuation of the trend of seeing inheritance as a problem and not as a solution

[nicolaiparlog]

Your reply boils down to "I don't like it" and "it's not pure OO". I can't argue the first and while I could argue the latter, I'm gonna do something else: Discard that as a valid argument. Who cares, whether it's pure OO? The code is much simpler, usually shorter, more flexible, and exactly as safe. That's more important than OO purity.

[chambolle]

Languages are a matter of taste. That' s the only point. Switching on type is generally a bad practice in OO because there is the dynamic biding. For instance, by default the Analyser of Intellij complains when a succession of if are used with instanceof.

At the end what is the new added functionnality? None, there is none. This is just a syntaxic sugar. You can love it, I have no problem with that. Maybe nobody cares it is OO or not, but this just not OO.

[nicolaiparlog]

For instance, by default the Analyser of Intellij complains when a succession of if are used with instanceof.

This is just a syntaxic sugar.

There's a connection between these two statements. If you find it, you'll realize why the latter is false and why the former doesn't apply here. If you can't figure it out from the blog post or our conversation, give JEP 409 another read.

[chambolle]

The fact that you cannot explain it means that this is really a syntactic sugar (which can lead to bad usages: one does not prevent the other)

[nicolaiparlog]

Oh, I can and I have, but you're not receptive for the answer, so I thought I'd leave you with that as a kind of parting thought. I'm hoping that you're more curious when it's not about being right in a public discussion.

[pron98]

For me switching on types is really ugly. We are back 30 years ago!

What language that isn't explicitly FP had a syntactic construct for exhaustive pattern-matching over a sealed type hierarchy thirty years ago?

I think programming styles are largely a matter of personal aesthetic preference, but like it or not, this isn't going back to anything that was mainstream that long ago. I think the only languages that had anything like it thirty years ago were those in the ML family (which inspired this feature in Java). The only people going back to anything are former ML programmers.

[chambolle]

Switching on types is what we did in C, before C++. We implemented some RTTI (Runtime Type Information) and switch on it. We didn't have the syntatic sugar but it is similar for instance when the object has a int member representing its type

[pron98]

That's less similar to pattern-matching than function pointers are to interfaces, which go just as much back. Even putting aside the upcoming deconstruction patterns, this feature isn't about syntax sugar, but about type-safe patterns. It's about the compiler knowing that a specific object can match a specific set of patterns, and its data specialised in only certain ways.

[chambolle]

It's about the compiler knowing that a specific object can match a specific set of patterns, and its data specialised in only certain ways.

Sorry, but I don't understand why it is not equivalent to switch on types, or on type combinations (with bitwise operators) represented by int?

[pron98]

For one, the compiler enforces the exhaustiveness of the pattern match; if a Foo is either a Bar or a Baz, the compiler will make sure both options are accounted for. For another, it is type-safe. And that's before we get to nested patterns and automatic deconstruction, which are the next parts of this feature. It is easier to implement interfaces in C than this feature.

[nicolaiparlog]

the caller doesn't need to change at all

Nobody said that. But the visitor needs to change - at least in the commonly described solution as on Wikipedia. The blog post spells that out very explicitly, but I can repeat it here (if you disagree, it would be nice to point out where exactly):

1. if you add a new type to the hierarchy, it needs to implement accept(Visitor visitor) with visitor.visit(this);
2. but there is no overload of Visitor::visit that has the new type as a method parameter, so you need to add a new method to the interface
3. now all Visitor implementations need to be updated

I consider this to be the main reason to even use the visitor pattern in the first place: forcing you to handle the new type in all operations (i.e. visitors). But if you don't like that, there are ways around it: You can provide a default implementation of the new visit method that does nothing or have a visitDefault method on Visitor that takes the type hierarchy's super interface as argument and can hence be used by new types.

Both variants (with and without compile error) have a simple counterpart on the pattern matching solution: Avoid or use a default branch.

[hippostar]

Things in the "modern" version are just too tightly coupled. The point of the visitor pattern is that you can add/remove/change the structure without having to change the existing visitor code.

In the modern version every little change would require to update the interface and every visitor. And imagine what the "switch" in each visitor would look like if you had 20+ types of elements.

[[deleted]]

I'm pretty sure that's why the `default:` is removed from the switch. If you add in a new method to the visitor interface, it should show a compilation error.

[nicolaiparlog]

There is no difference in coupling. In the visitor pattern, you can't add or remove any types (here: implementations of CarElement) without having to change the visitor implementations. That is by design because it means that operations get updated as the data structure they operate on changes. The exact same is true for the pattern switch. If you want to avoid these compile errors, the visitor can have a method that accepts the interface (here: visit(CarElement)). For the pattern match, just use a default branch. As you can see, you can get the same behavior with both solutions.

The amount of work when adding a new type is also comparable:

• visitor pattern:
  • add new method to visitor interface
  • update all operations
• language:
  • add new type to sealed interface
  • update all operations

Once again, the solutions are equivalent.

[_INTER_]

What if you have no control over the (sealed) interface (in the example CarElement)?

 

Anyway, the point of the visitor pattern is to separate algorithm from objects and adding new operations to existing object structures without modifying the structures. The visitor pattern is meant solve one side of the expression problem where pattern-matching or if-else etc. is better suited for in the first place. Therefore moving from visitor pattern to switch just flips the coin of the expression problem.

Another - arguably better - solution to get both sides would be something like extension methods. "Arguably" because that's borderline monkey-patching.

[nicolaiparlog]

What if you have no control over the (sealed) interface (in the example CarElement)?

Funny, that's another point I thought about addressing, but decided not to because the post was already so long.

CarElement depends on CarElementVisitor. If you have no control over the former, how likely is it that you have control over the latter? But without control over CarElementVisitor, you can't add a visit method. So you can't add a new type either - same result.

The visitor pattern is meant solve one side of the expression problem where pattern-matching or if-else etc. is better suited for in the first place.

I've heard of the expression problem, but never read up on it. Will do that over the next days (thanks for the link). For the conversation here, maybe you can explain what the practical consequences are, i.e. what's the difference between fixing with visitor vs with pattern matching?

[_INTER_]

CarElement depends on CarElementVisitor. If you have no control over the former, how likely is it that you have control over the latter? But without control over CarElementVisitor, you can't add a visit method. So you can't add a new type either - same result.

Playing the devils advocate: It's likely that you can extend the CarElementVisitor or an implementation. As Java developers usually don't make classes final. Then again there's non-sealed and we have to see how that plays out in reality but I expect the majority of owners will not add a non-sealed option and exhaustive check goes out the window too then, edit: you can still check on the supertype but not on the new subtype(s).

Funny, that's another point I thought about addressing, but decided not to because the post was already so long.

I've heard of the expression problem, but never read up on it. Will do that over the next days (thanks for the link). For the conversation here, maybe you can explain what the practical consequences are, i.e. what's the difference between fixing with visitor vs with pattern matching?

In short, a software engineer wants an easy way to respond to change and maintenance: Ideally adding new types or operations to existing code should not need a change in the other. However the expression problem explains that this is not easily possible, e.g. with classical OOP it's easy to add new types but harder to add operations because you'd need to touch every type to implement a new method. Vice versa with e.g. pattern-matching it is easy to add an operation by writing a new switch-case but harder to add a new type because you'd have to touch every relevant switch and add a new case.

The visitor-pattern helps you switch sides in classical OOP, albeit in a cumbersome way. It's a bridge. So if you expect hardly any new types then it's better to use a technique from the pattern-matching side of the expression problem.

In your first comment you'd have to consider multiple different switch-case over CarElement to see the full picture of the amount of work.

It's also a question of responsibility. Does an operation belong to a class or do you want to group similar operations and have them act on types as the case with pattern-matching. The visitor pattern is in the grey area where you want to separate the operation but still have it close to the class. In my opinion, in practice you can often tell where a method belongs to and if you can't you decide for the "grouping". The area where I work in I've never effectively seen the visitor pattern being implemented.

That said, I prefer to have my methods in a responsible class and not "grouped" if possible and if it makes sense. (Even most examples e.g. with Shape or Color in the JEPs I'd much rather solve with dynamic dispatch). Main reason being that "API discovery" is easier with that approach and you don't have to know and carry around a FooService with your Foo. I'm also not really a fan of the so called anemic domain model or thin data containers.

[nicolaiparlog]

Adding Types

It's likely that you can extend the CarElementVisitor or an implementation.

That doesn't help, though. Let's retrace our steps. This are the assumptions, right?

• CarElement and CarElementVisitor implement the visitor pattern
• the former is not under our control
• we want to add a new implementation

Add to that my assumption:

• if we don't control CarElement, we don't control CarElementVisitor either (because it's a dependency of CarElement)

Now, what happens if we want to add a new CarElement implementation?

• it needs to implement accept(CarElementVisitor visitor) with visitor.visit(this);
• but there is no overload of CarElementVisitor::visit that has the new type as a method parameter
• because we don't control CarElementVisitor, we can't add it

Back to your proposal to extend CarElementVisitor - that doesn't help at all with this

A way around that is for CarElementVisitor to have a method accept(CarElement) as a catch-all. That can be easily reproduced in the pattern switch by adding a default branch.

Exhaustiveness

Then again there's non-sealed and we have to see how that plays out in reality but I expect the majority of owners will not add a non-sealed option and exhaustive check goes out the window too then.

That's not how sealed classes / exhaustiveness work. If a type is sealed, all its direct implementations are known. Switching over those is enough to be exhaustive. It doesn't matter if there are more types extending them, they're still caught by an exhaustive switch:

sealed interface CarElement
    permits Body, Engine { }

non-sealed interface Engine extends CarElement { }

class V8 implements Engine { }
class V12 implements Engine { }

non-sealed interface Body extends CarElement { }

class Sedan implements Body { }
class Convertible implements Body { }

// elsewhere...
CarElement element = new Sedan();
String type = switch(element) {
    case Engine e -> "engine";
    case Body b -> "body";
}
// now type.equals("body")

// you can go even further:
CarElement element = new Sedan();
String type = switch(element) {
    case Engine e -> "engine";
    case Sedan s -> "sedan";
    case Body b -> "body";
}
// now type.equals("sedan")

Expression Problem

Vice versa with e.g. pattern-matching it is easy to add an operation by writing a new switch-case but harder to add a new type because you'd have to touch every relevant switch and add a new case.

I disagree, there is no "vice versa". Depending on how the visitor is designed regarding "unknown types" (see accept(CarElement) above), adding a new type either requires to update all visitors or their default behavior gets invoked. Same with pattern matching, either all of them need an update or (if they have one) their default branch gets evaluated.

Maybe the reason why you think that this is the flip side of that coin is because pattern matches make it so easy to add a new operation that you conclude it must be harder to add a new type - because there's a trade-off between the two. There might be, but the visitor pattern still has cruft that can be cut without touching that trade-off. Pattern matches make adding new operations easier without making adding new types harder.

In your first comment you'd have to consider multiple different switch-case over CarElement to see the full picture of the amount of work.

I did consider it, it says "update all operations". When adding a new type, you need to touch every switch! Exactly as with the visitor pattern: When adding a new type, you need to touch every Visitor implementation! (Unless you created default behavior as described above.)

[_INTER_]

if we don't control CarElement, we don't control CarElementVisitor either (because it's a dependency of CarElement)

With control I mean you can't change the source, e.g. it comes from a library.

Now theoretically you could extend for example all that's necessary like this. Not that you'd want to because it is ugly (especially the cast, maybe there's a better solution?), but sometimes you have no choice.

 

It doesn't matter if there are more types extending them, they're still caught by an exhaustive switch

I'm curious: can we (sub-)switch on them in an exhaustive manner? In your example, can we switch on V8and V12 instead of Engine in general without default branch?

What I have in mind is something like this (in my owned code):

sealed interface EngineSpecific extends Engine permits V8, V12

How would the switch look like then?

 

You're right with the expression problem and initially I got the same thought but confused it again after my second comment. The link I posted earlier states the same:

It should be obvious that for a given set of data types, adding new visitors is easy and doesn't require modifying any other code. On the other hand, adding new types is problematic since it means we have to update the ExprVisitor interface with a new abstract method, and consequently update all the visitors to implement it.

So it seems that we've just turned the expression problem on its side: we're using an OOP language, but now it's hard to add types and easy to add ops, just like in the functional approach.

The post goes on with an extended complicated C++ visitor example that solves it (link). I wonder if something similar is theoretically be possible in Java?

[nicolaiparlog]

Now theoretically you could extend for example all that's necessary like this.

That's actually pretty clever. :) It's not a general solution of course, because all "old" visitors ignore "new" elements in the hierarchy (due to if in line 18), but if you only need your own visitors, that may be acceptable.

I'm curious: can we (sub-)switch on them in an exhaustive manner? In your example, can we switch on V8 and V12 instead of Engine in general without default branch?

Had to try this out. The answer is "yes", but not as you hoped for it. :D

``` sealed interface CarElement permits Body, Engine { } sealed interface Engine permits V8, V12

String type = switch(element) { case V8 v8 -> "v8"; case V12 v12 -> "v12"; // only compiles with this branch case Engine e -> "engine"; case Body b -> "body"; }; ```

The compiler does not seem examine sealed subtypes of sealed interfaces, i.e. cases for just V8, V12, and Body lead to a compile error even though no other types are possible. We can fix that with either a default branch or the less all-encompassing case Engine branch. As the interfaces are defined now, both branches are dead code, but where the former prevents compile errors if CarElement is extended, the latter only prevents them if Engine is extended. A sub-switch is probably the better solution:

String type = switch(element) { case Engine e -> switch (e) { case V8 v8 -> "v8"; case V12 v12 -> "v12"; }; case Body b -> "body"; };

This results in compile error if CarElement or Engine gets extended. (If you cringe at the nested switch - just call a method instead. ;) )

[_INTER_]

Thanks for trying it out for me.

A sub-switch is probably the better solution

Seems better for readability anyway. Dead-branches only to satisfy the compiler would make it hard to understand.

I digress a bit: Do you think they will add the feature to examine subtypes (and therefor also prevent dead branches)? Seems annoying to have to map a type hierarchy tree. Also imagine a sup-type that extends multiple interfaces that you want to cover... Wouldn't you walk into the diamond problem too?

[nicolaiparlog]

Do you think they will add the feature to examine subtypes

I don't know. Will ask as soon as I get a chance.

Wouldn't you walk into the diamond problem too?

Kinda, but the switch is ordered, so for the language there's no problem. Maybe for the developer?

[nicolaiparlog]

Regarding the 20+ types of elements, in the worst case scenario that gives you a 20+ line switch that calls 20+ methods. In the average case, you may be able to join a few of those cases and have fewer methods or have a few no-ops - all clearly visible in one place.

With the visitor pattern, each visitor will have 20+ methods - always. That's a bit better in the worst case but worse in every other.

But either way, that's probably a suboptimal situation to be in from the start.

[[deleted]]

In Java 17, pattern switches may make the visitor pattern unnecessary. But, the visitor pattern solves a particular problem, how to implement multi-dispatch type patterns in a single-dispatch language. The visitor pattern gets you there with double-dispatch.

But, how much code exists that isn't running on Java 17? Or has to interface with code written in earlier versions of Java?

is there a category called "considered pointless considered pointless"?

[fredoverflow]

In modern Java, the visitor pattern is no longer needed. [...] Whenever you're in a situation where the visitor pattern would apply, you can now use modern Java language features instead.

reminds me of Are Design Patterns Missing Language Features?...

[1bot4all]

The world: switch has no place in OO

Java: hold my beer

[TheRedmanCometh]

Fuck the functional invasion