Skip to content

Instantly share code, notes, and snippets.

@nuttycom
Last active January 8, 2017 21:06
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save nuttycom/6e286855bc67b3d89096824eba33d531 to your computer and use it in GitHub Desktop.
Save nuttycom/6e286855bc67b3d89096824eba33d531 to your computer and use it in GitHub Desktop.
interface ShapeVisitor<A>
{
// The implementation of ApplyRect defines handling for the
// operation defined by this visitor in the case that the
// shape is a rectangle.
public A ApplyRect(IRectangle rect);
// The implementation of ApplyCircle defines handling for the
// operation defined by this visitor in the case that the
// shape is a circle.
public A ApplyCircle(ICircle circle);
}
public class AreaVisitor : ShapeVisitor<double>
{
public double ApplyRect(IRectangle rect)
{
return rect.height * rect.width;
}
public double ApplyCircle(ICircle circle)
{
return Math.PI * circle.radius * circle.radius ;
}
}
public class PerimeterVisitor: ShapeVisitor<double>
{
public double ApplyRect(IRectangle rect)
{
return 2 * (rect.height + rect.width);
}
public double ApplyCircle(ICircle circle)
{
return Math.PI * circle.radius * 2;
}
}
abstract class IShape {
public A accept<A>(ShapeVisitor<A> visitor);
// This becomes essentially syntactic sugar; it need not be defined as a method on the interface.
public double GetArea()
{
return accept(new AreaVisitor());
}
// I will not define GetPerimeter here - there is no purpose. The user
// can simply call myShape.accept(new PerimeterVisitor()) if they
// wish that functionality. The point here is that the end user can "extend"
// the interface by adding new Visitor implementations - they do not need
// IShape to be altered.
}
class ICircle : IShape {
public A accept<A>(ShapeVisitor<A> visitor) {
return visitor.ApplyCircle(this);
}
}
class IRectangle: IShape {
public A accept<A>(ShapeVisitor<A> visitor) {
return visitor.ApplyRect(this);
}
}
@giuliohome
Copy link

Hi! Now I see that GetArea() is accept() and I see that accept is a method of the interface IShape but - sorry to bother you - how is accept implemented in the concrete Shape class? Am I missing something again?

@giuliohome
Copy link

Oh, you edited your comment saying that you would rely upon end users - and that's where our view points and assumptions start to differ

@giuliohome
Copy link

And the “expression problem” in OO vs FP can be part of the issue we’re discussing indeed, like I can find here http://stackoverflow.com/a/3776140

@giuliohome
Copy link

giuliohome commented Jan 3, 2017

In my understanding you want to use 2 different overloads of the GetArea, that are differentiated only by the circle/rectangle input interface.
If so, they should have the same name.
Nonetheless I see your point: you can force an apparently stronger recognition, by starting from an inherited, own property, instead of an external case... but don't get distracted by the specific example.
One can always have a "bug-fix" or an "enhancement" simply because we have to switch the Greek pi from one to the other formula... and again don't just think about the basic Euclidean geometry, but imagine an algorithm for new dimensions in a strange Riemann manifold for the latest string theories of sort ;))

@nuttycom
Copy link
Author

nuttycom commented Jan 3, 2017

@giuliohome: with respect to the overloading of the Visit* (here called Apply*) methods, I don't think that overloading helps or clarifies anything; in fact, I think that the traditional Visitor pattern recommends overloading misses the point. For example, you can use visitor in dynamic languages, but there you do not have the ability to overload on the argument type. In general, I think overloading is a misfeature; in working in Scala I've learned to live without it, much to improved clarity of my code. Here, I think that overloading would be particularly problematic, because it would be essentially invisible in the implementation of the Accept* methods.

@giuliohome
Copy link

giuliohome commented Jan 4, 2017

Ok. Thank you! Now your proposal is sufficiently clear to me.

I like your code and I've nothing against it at first sight. In the context of my blog's article, I was trying to show a more natural C# implementation than a complex machinery to render F# algebraic sum types into C# - and also your code could be viewed as an example of what would more naturally fit C#... But, for the sake of finding some negative aspects... nothing prevents one from writing the following code (actually I haven't tried, so I'm not sure it compiles, but you can just figure it out...)

IShape circle = (IShape)this;
IRectangle error = (IRectangle)circle;
return visitor.ApplyRect(error);

@giuliohome
Copy link

giuliohome commented Jan 4, 2017

Well, one can not immediately convert from the base to the derived class (but usually ISomething means an interface, not a class), yet there are many reasons why one may want to do it and it is doable in several ways, like via constructor, etc...

@giuliohome
Copy link

And again, if I remember correctly, the above would cause a runtime, not a compilation error.

@giuliohome
Copy link

giuliohome commented Jan 4, 2017

Other "practical" examples... (but real world applications are much more complex than this trivial geometric model)

  1. you're asked to refactor rectangle to parallelogram => you have to manually rename applyRect to applyParal... to preserve a meaningful naming convention...
  2. you're then asked to add also rectangle besides parallelogram => now you don't want to maintain 2 different methods (ApplyRect, ApplyParal...) for the same (height * width) area's formula ... and that would be easier with my interface IRectangle, irrespective of whether you have a rectangle derived class or a parallelogram class or whatever. Of course you can do the same with the Visitor ApplySomething(ISomeInterface someInterface) but you need to start to do at least an implicit type conversion, etc...

@nuttycom
Copy link
Author

nuttycom commented Jan 4, 2017

@giuliohome: with respect to your casting example, of course - if you circumvent the type system via a cast, then the type system cannot help you. But, with techniques like the Visitor pattern as shown here, it's essentially never necessary or desirable to use casts. For my part, in the past decade of coding in typed languages (relevant ones being Java and Scala) using these techniques, I have not written a single cast.

Now, for the subesquent questions:

  1. That renaming is fairly trivial and mechanical, but I see your point. Nonetheless, I still tend to prefer distinct names to overloading for pedagogical purposes - if someone who hasn't encountered the technique before sees both calls named simply 'apply' and overloaded on the type, they may incorrectly assume that overloading is somehow intrinsically part of the pattern, which is not the case. In fact, I think that the GOF book's definition of Visitor explicitly refers to overloading as being part of the pattern, which is misleading.

  2. To answer this one properly, I'm going to provide a similar example in my preferred language, which is Haskell:

data Shape
  = Circle { radius :: Double }
  | Rectangle { height :: Double, width :: Double }

In the case that Rectangle is refactored to parallellogram:

data Shape
  = Circle { radius :: Double }
  | Parallelogram { height :: Double, width :: Double, acuteAngle :: Double }

It is highly useful to get a compilation error in each function that takes a Shape as an argument, for the simple reason that some assumptions that held true for rectangles may no longer hold true for parallellograms. While the area formula may remain the same, other important properties (say length of diagonal) will not, and having the compiler alert you to places where you should consider whether the current implementation still obtains the correct result.

@nuttycom
Copy link
Author

nuttycom commented Jan 4, 2017

Also: if instead you added Parallelogram as a separate case, rather than replacing Rectangle and wished to reuse the implementation, the appropriate thing to do is to factor that implementation out to a function of two arguments, rather than somehow attempt to reuse the innards of the 'ApplyRect' implementation. In short, what 'ApplyRect' provides you is simply the knowledge that you're able to invoke .height, .width, .accept and no other methods in this context.

@nuttycom
Copy link
Author

nuttycom commented Jan 4, 2017

The whole point of using a type system is to exclude invalid states from the state space of your application by construction. In languages like C#, we have built-in product types (read: objects) which readily multiply the size of our state space, but lack built-in sum types to help us reduce it. This is a grave error; Visitor helps us to mitigate it. For additional context, here's another talk that I gave a few years ago on the subject: https://github.com/nuttycom/lambdaconf-2014/blob/gh-pages/slides.md

@giuliohome
Copy link

giuliohome commented Jan 4, 2017

"... the appropriate thing to do is to factor that implementation out to a function of two arguments" Well, I was waiting for this answer. That function of two arguments is a backdoor where the benefits of the visitor can be possibly lost. Because it is a function of two doubles and now it can be wrongly replaced by another function with the same signature (let me fantasize that we want a common function of r1,r2 for circle and ellipse, degenerating in the circle's area for r1 = r2).
Now, if you compare this logical error (erroneously switching the common underlying function) - that will silently give a wrong result in production - to the runtime exception due to a wrong type conversion (I'd say that an explicit cast is not even needed), maybe the latter could be better in term of ensuring the consistency of a "valid" result... So, while a certain programming style doesn't prevent you from a runtime exception, this behavior could simply represent the more likely outcome of an underlying logical problem, thus really helping to spot a hidden issue.

Enough of this all.
I concede that the visitor pattern can be useful indeed, as any other design pattern obviously :-)

Will read your talk, I'm sure it will be interesting and challenging.
Good luck for your work. My pleasure to have virtually met you here ;)

@nuttycom
Copy link
Author

nuttycom commented Jan 5, 2017

Heh. It's true - Double is a terrible type to use. Much better to use distinct Width and Height newtype wrappers - as you might see a discussion of in my talk!

@giuliohome
Copy link

giuliohome commented Jan 8, 2017

Real word apps are full of types like double, string, etc and they're useful when you have to deliver an app within a deadline and you don't have time to add tons of code without any additional feature from the user perspective. Anyway, there is another point and it is related to the state handling. Let me explain it below.

Oh ... and this is a big problem: with a switch statement I can manage a mutable state IShape that is now Rectangle and then Circle. I don't see how it could be done with a Visitor, since I should accept a generic IShape and that's against the Visitor pattern in the first place. Have a look at my latest repository and tell me if you see any way - using C# as a language - to replace this switch with a Visitor?

Thank you again!

@giuliohome
Copy link

Never mind! Found and commited the changes to switch to a visitor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment