Double Dispatch is a Code Smell

If you’re using Double Dispatch in your code, this may be a symptom of an underlying design issue which may impact the maintainability of your application.  Due to the fact that Double Dispatch is at times confused with a form of the Strategy Pattern, an overview may be in order to elaborate on this assertion further.

 

What is Double Dispatch?

Technically, Double Dispatch refers to a technique used in the context of a polymorphic method call for mitigating the lack of multimethod support in programming languages.  More simply, Double Dispatch is used to invoke an overloaded method where the parameters vary among an inheritance hierarchy.  To explain fully, let’s start with a review of polymorphism.

 

Polymorphism

In the following example, a hierarchy of shapes are defined with each of the derived types overloading a base virtual Draw() method.  Next, a console application is used to define a list of each of the shapes and iterate over each shape in the collection calling the Draw() method of each item in the list:

    class Shape
    {
        public virtual void Draw()
        {
            Console.WriteLine("A shape is drawn.");
        }
    }

    class Polygon : Shape
    {
        public override void Draw()
        {
            Console.WriteLine("A polygon is drawn.");
        }
    }

    class Quadrilateral : Polygon
    {
        public override void Draw()
        {
            Console.WriteLine("A quadrilateral is drawn.");
        }
    }

    class Parallelogram : Quadrilateral
    {
        public override void Draw()
        {
            Console.WriteLine("A parallelogram is drawn.");
        }
    }

    class Rectangle : Parallelogram
    {
        public override void Draw()
        {
            Console.WriteLine("A rectangle is drawn.");
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var shapes = new List<Shape>
                             {
                                 new Shape(),
                                 new Polygon(),
                                 new Quadrilateral(),
                                 new Parallelogram(),
                                 new Rectangle()
                             };

            foreach (Shape shape in shapes)
            {
                shape.Draw();
            }

            Console.ReadLine();
        }
    }

The following lines are printed to the console upon running the application:

A shape is drawn.
A polygon is drawn.
A quadrilateral is drawn.
A parallelogram is drawn.
A rectangle is drawn.


Note that the proper Draw() method is called for each item in the collection.  In most object-oriented languages, this polymorphic behavior is achieved through the use of a virtual table consulted at run-time to derive the proper offset address for an object’s method.  This behavior is referred to as "Dynamic Dispatch" or "Single Dispatch".  So, how does this relate to Double Dispatch?  To answer this question, let’s next review method overloading.

 

Method Overloading

In the following example, our Shape class is redefined to have two overloaded Draw methods: one with a parameter of type Surface and one with a parameter of type EtchASketch:

    class Surface
    {
    }

    class EtchASketch : Surface
    {
    }

    class Shape
    {
        public void Draw(Surface surface)
        {
            Console.WriteLine("A shape is drawn on the surface with ink.");
        }

        public void Draw(EtchASketch etchASketch)
        {
            Console.WriteLine("The knobs are moved in attempt to draw the shape.");
        }
    }
  
    class Program
    {
        static void Main(string[] args)
        {
            var shape = new Shape();
            shape.Draw(new Surface());
            shape.Draw(new EtchASketch());

            Console.ReadLine();
        }
    }

When executed, the following lines are printed to the console:

A shape is drawn on the surface with ink.
The knobs are moved in attempt to draw the shape.


Note that the parameter type determines which Draw() method is invoked. 


But what happens if we change the Main() method to the following?

    class Program
    {
        static void Main(string[] args)
        {
            var shape = new Shape();
            Surface surface = new Surface();
            Surface etchASketch = new EtchASketch();

            shape.Draw(surface);
            shape.Draw(etchASketch);

            Console.ReadLine();
        }
    }

Executing this produces the following:

A shape is drawn on the surface with ink.
A shape is drawn on the surface with ink.


What happened?  The issue here is that the method to call was determined statically at compile time based upon the reference type, not at run-time based upon the object type.  To resolve this issue, another technique is needed … Polymorphic Static Binding.

 

Polymorphic Static Binding

Polymorphic static binding is a technique where static method invocations are determined at run-time through the use of polymorphism.  This can be demonstrated in our example by adding a new Draw(Shape shape) method to the Surface and EtchASketch types which call shape.Draw() with a reference to the current object:

    class Surface
    {
        public virtual void Draw(Shape shape)
        {
            shape.Draw(this);
        }
    }

    class EtchASketch : Surface
    {
        public override void Draw(Shape shape)
        {
            shape.Draw(this);
        }
    }

To invoke the correct Shape.Draw() method, our console application needs to be modified to call the the method indirectly through a Surface reference:

    class Program
    {
        static void Main(string[] args)
        {
            var shape = new Shape();
            Surface surface = new Surface();
            Surface etchASketch = new EtchASketch();

            surface.Draw(shape);
            etchASketch.Draw(shape);

            Console.ReadLine();
        }
    }

Upon executing the application again, the following lines are now printed:

A shape is drawn on the surface with ink.
The knobs are moved in attempt to draw the shape.


This example achieves the desired result by effectively wrapping the static-dispatched method invocation (i.e. Shape.Draw()) within a virtual-dispatch method invocation (i.e. Surface.Draw() and EtchASketch.Draw()).  This causes the static Shape.Draw() method invocation to be determined by which virtual Surface.Draw() method invocation is executed.


Although the above example now contains a method invocation using a reference to the current object as the method parameter (often seen with Double Dispatch), it should be noted that Double Dispatch has yet to be demonstrated.  Thus far, only one level of virtual dispatching has been used.  To demonstrate Double Dispatch, the techniques from both the polymorphism example and the polymorphic static binding example need to be combined as seen in the next section.

 

Double Dispatch

The following example contains a hierarchy of Surface types and a hierarchy of Shape types.  Each Shape type contains an overloaded virtual Draw() method which contains the logic for how the shape is to be drawn on a particular surface.  The example console application uses the polymorphic static binding technique to ensure the proper overload is called for each surface type:

    class Surface
    {
        public virtual void Draw(Shape shape)
        {
            shape.Draw(this);
        }
    }

    class EtchASketch : Surface
    {
        public override void Draw(Shape shape)
        {
            shape.Draw(this);
        }
    }

    class Shape
    {
        public virtual void Draw(Surface surface)
        {
            Console.WriteLine("A shape is drawn on the surface with ink.");
        }

        public virtual void Draw(EtchASketch etchASketch)
        {
            Console.WriteLine("The knobs are moved in attempt to draw the shape.");
        }
    }

    class Polygon : Shape
    {
        public override void Draw(Surface surface)
        {
            Console.WriteLine("A polygon is drawn on the surface with ink.");
        }

        public override void Draw(EtchASketch etchASketch)
        {
            Console.WriteLine("The knobs are moved in attempt to draw the polygon.");
        }
    }

    class Quadrilateral : Polygon
    {
        public override void Draw(Surface surface)
        {
            Console.WriteLine("A quadrilateral is drawn on the surface with ink.");
        }

        public override void Draw(EtchASketch etchASketch)
        {
            Console.WriteLine("The knobs are moved in attempt to draw the quadrilateral.");
        }
    }

    class Parallelogram : Quadrilateral
    {
        public override void Draw(Surface surface)
        {
            Console.WriteLine("A parallelogram is drawn on the surface with ink.");
        }

        public override void Draw(EtchASketch etchASketch)
        {
            Console.WriteLine("The knobs are moved in attempt to draw the parallelogram.");
        }
    }

     class Rectangle : Parallelogram
    {
        public override void Draw(Surface surface)
        {
            Console.WriteLine("A rectangle is drawn on the surface with ink.");
        }

        public override void Draw(EtchASketch etchASketch)
        {
            Console.WriteLine("The knobs are moved in attempt to draw the rectangle.");
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            Surface surface = new Surface();
            Surface etchASketch = new EtchASketch();

            var shapes = new List<Shape>
                             {
                                 new Shape(),
                                 new Polygon(),
                                 new Quadrilateral(),
                                 new Parallelogram(),
                                 new Rectangle()
                             };

            foreach (Shape shape in shapes)
            {
                surface.Draw(shape);
                etchASketch.Draw(shape);
            }

            Console.ReadLine();
        }
    }

Executing this example produces the following:

A shape is drawn on the surface with ink.
The knobs are moved in attempt to draw the shape.
A polygon is drawn on the surface with ink.
The knobs are moved in attempt to draw the polygon.
A quadrilateral is drawn on the surface with ink.
The knobs are moved in attempt to draw the quadrilateral.
A parallelogram is drawn on the surface with ink.
The knobs are moved in attempt to draw the parallelogram.
A rectangle is drawn on the surface with ink.
The knobs are moved in attempt to draw the rectangle.


In the above example, virtual dispatch occurs twice for each call to one of the Surface references: Once when the Surface.Draw() virtual method is called and again when either calls the Shape.Draw() overloaded virtual method.  Note again that while the second virtual dispatch is based on the type of Shape instance, the overloaded method called is still determined statically based upon the reference type.

 

Consequences

So, what’s wrong with Double Dispatch?  The problem isn’t so much in the technique, but what design choices might be leading to reliance upon the technique.  Consider for instance the hierarchy of shape types in our Double Dispatch example.  What happens if we want to add a new surface?  In this case, each of the shape types will need to be modified to add knowledge of the new Surface type.  This violates the Open/Closed Principle, and in this case in a particularly egregious way (i.e. It’s violation is multiplied by the number of shape types we have).   Additionally, it violates the Single Responsibility Principle.  Changes to how shapes are drawn on a particular surface are likely to differ from surface to surface, thereby leading our shape objects to change for different reasons.

The presence of Double Dispatch generally means that each type in a hierarchy has special handling code within another hierarchy of types.  This approach to representing variant behavior leads to code that is less resilient to future changes as well as being more difficult to extend.

 

The Matrix: Reloaded

Let’s take another stab at modeling our shape/surface intersection matrix.  In the following example, several new concepts have been introduced to facilitate decoupling: line segments, points, and brushes:

    interface ISurface
    {
        void Add(LineSegment segment);
    }

    class Paper : ISurface
    {
        readonly IList<LineSegment> _segments = new List<LineSegment>();

        public void Add(LineSegment segment)
        {
            _segments.Add(segment);
        }
    }

    class EtchASketch : ISurface
    {
        readonly IList<LineSegment> _segments = new List<LineSegment>();

        public void Add(LineSegment segment)
        {
            _segments.Add(segment);
        }
    }

    class Point
    {
        public Point(int x, int y)
        {
            X = x;
            Y = y;
        }

        public int X { get; set; }
        public int Y { get; set; }
    }

    class LineSegment
    {
        public LineSegment(Point point1, Point point2)
        {
            Point1 = point1;
            Point2 = point2;
        }

        public Point Point1 { get; set; }
        public Point Point2 { get; set; }
    }

    interface IShape
    {
        IList<LineSegment> GetLineSegments();
    }

    class Polygon : IShape
    {
        public IList<LineSegment> GetLineSegments()
        {
            var segments = new List<LineSegment>();
            segments.Add(new LineSegment(new Point(0, 0), new Point(0, 9)));
            segments.Add(new LineSegment(new Point(0, 9), new Point(3, 6)));
            segments.Add(new LineSegment(new Point(3, 6), new Point(6, 9)));
            segments.Add(new LineSegment(new Point(6, 0), new Point(6, 9)));
            segments.Add(new LineSegment(new Point(6, 0), new Point(3, 3)));
            segments.Add(new LineSegment(new Point(3, 3), new Point(0, 0)));

            return segments;
        }
    }

    class Quadrilateral : IShape
    {
        public IList<LineSegment> GetLineSegments()
        {
            var segments = new List<LineSegment>();
            segments.Add(new LineSegment(new Point(0, 0), new Point(0, 9)));
            segments.Add(new LineSegment(new Point(0, 9), new Point(4, 5)));
            segments.Add(new LineSegment(new Point(4, 0), new Point(0, 4)));
            segments.Add(new LineSegment(new Point(4, 0), new Point(0, 0)));

            return segments;
        }
    }

    class Parallelogram : IShape
    {
        public IList<LineSegment> GetLineSegments()
        {
            var segments = new List<LineSegment>();
            segments.Add(new LineSegment(new Point(0, 4), new Point(0, 9)));
            segments.Add(new LineSegment(new Point(0, 9), new Point(4, 5)));
            segments.Add(new LineSegment(new Point(4, 0), new Point(4, 5)));
            segments.Add(new LineSegment(new Point(4, 0), new Point(0, 4)));

            return segments;
        }
    }

    class Rectangle : IShape
    {
        public IList<LineSegment> GetLineSegments()
        {
            var segments = new List<LineSegment>();
            segments.Add(new LineSegment(new Point(0, 0), new Point(0, 9)));
            segments.Add(new LineSegment(new Point(0, 9), new Point(9, 4)));
            segments.Add(new LineSegment(new Point(4, 0), new Point(9, 4)));
            segments.Add(new LineSegment(new Point(4, 0), new Point(0, 0)));

            return segments;
        }
    }

    class Program
    {
        static readonly IDictionary<Type, IBrush> brushDictionary = new Dictionary<Type, IBrush>();

        static Program()
        {
            brushDictionary.Add(typeof (Paper), new Pencil());
            brushDictionary.Add(typeof (EtchASketch), new EtchASketchKnobs());
        }

        static void Main(string[] args)
        {
            var surfaces = new List<ISurface>
                               {
                                   new Paper(),
                                   new EtchASketch()
                               };

            var shapes = new List<IShape>
                             {
                                 new Polygon(),
                                 new Quadrilateral(),
                                 new Parallelogram(),
                                 new Rectangle()
                             };

            foreach (ISurface surface in surfaces)
                foreach (IShape shape in shapes)
                {
                    Console.WriteLine(string.Format("Drawing a {0} on the {1} ...", shape.GetType().Name,
                                                    surface.GetType().Name));
                    brushDictionary[surface.GetType()].Draw(surface, shape.GetLineSegments());
                    Console.WriteLine(Environment.NewLine);
                }

            Console.ReadLine();
        }
    }

    interface IBrush
    {
        void Draw(ISurface surface, IList<LineSegment> segments);
    }

    class Pencil : IBrush
    {
        public void Draw(ISurface surface, IList<LineSegment> segments)
        {
            foreach (LineSegment segment in segments)
            {
                Console.WriteLine(string.Format("Pencil used to sketch line segment {0},{1} to {2},{3}.",
                                                segment.Point1.X, segment.Point1.Y,
                                                segment.Point2.X, segment.Point2.Y));
            }
        }
    }

    class EtchASketchKnobs : IBrush
    {
        public void Draw(ISurface surface, IList<LineSegment> segments)
        {
            foreach (LineSegment segment in segments)
            {
                Console.WriteLine(string.Format("Knobs used to produce line segment {0},{1} to {2},{3}.",
                                                segment.Point1.X, segment.Point1.Y,
                                                segment.Point2.X, segment.Point2.Y));
            }
        }
    }

Executing this example produces the following:

Drawing a Polygon on the Paper ...
Pencil used to sketch line segment 0,0 to 0,9.
Pencil used to sketch line segment 0,9 to 3,6.
Pencil used to sketch line segment 3,6 to 6,9.
Pencil used to sketch line segment 6,0 to 6,9.
Pencil used to sketch line segment 6,0 to 3,3.
Pencil used to sketch line segment 3,3 to 0,0.


Drawing a Quadrilateral on the Paper ...
Pencil used to sketch line segment 0,0 to 0,9.
Pencil used to sketch line segment 0,9 to 4,5.
Pencil used to sketch line segment 4,0 to 0,4.
Pencil used to sketch line segment 4,0 to 0,0.


Drawing a Parallelogram on the Paper ...
Pencil used to sketch line segment 0,4 to 0,9.
Pencil used to sketch line segment 0,9 to 4,5.
Pencil used to sketch line segment 4,0 to 4,5.
Pencil used to sketch line segment 4,0 to 0,4.


Drawing a Rectangle on the Paper ...
Pencil used to sketch line segment 0,0 to 0,9.
Pencil used to sketch line segment 0,9 to 9,4.
Pencil used to sketch line segment 4,0 to 9,4.
Pencil used to sketch line segment 4,0 to 0,0.


Drawing a Polygon on the EtchASketch ...
Knobs used to produce line segment 0,0 to 0,9.
Knobs used to produce line segment 0,9 to 3,6.
Knobs used to produce line segment 3,6 to 6,9.
Knobs used to produce line segment 6,0 to 6,9.
Knobs used to produce line segment 6,0 to 3,3.
Knobs used to produce line segment 3,3 to 0,0.


Drawing a Quadrilateral on the EtchASketch ...
Knobs used to produce line segment 0,0 to 0,9.
Knobs used to produce line segment 0,9 to 4,5.
Knobs used to produce line segment 4,0 to 0,4.
Knobs used to produce line segment 4,0 to 0,0.


Drawing a Parallelogram on the EtchASketch ...
Knobs used to produce line segment 0,4 to 0,9.
Knobs used to produce line segment 0,9 to 4,5.
Knobs used to produce line segment 4,0 to 4,5.
Knobs used to produce line segment 4,0 to 0,4.


Drawing a Rectangle on the EtchASketch ...
Knobs used to produce line segment 0,0 to 0,9.
Knobs used to produce line segment 0,9 to 9,4.
Knobs used to produce line segment 4,0 to 9,4.
Knobs used to produce line segment 4,0 to 0,0.


By changing the Shape objects to be defined in terms of line segments, knowledge is removed from the shape concerning how to draw itself on any particular surface.  Additionally, the Surface type now encapsulates a collection of line segments to simulate the lines being drawn onto the surface.  To handle drawing the line segments onto the surfaces, we’ve introduced a Brush type which "draws" the line segments onto a surface in its own peculiar way.  To configure which brushes are to be used with which surface, the console application defines a dictionary matching surfaces to brushes.


In contrast to the Double Dispatch example, none of the existing types need to be modified to add new surfaces, shapes, or brushes.

 

Conclusion

Since Double Dispatch is a technique for calling virtual overloaded methods based upon parameter types which exist within an inheritance hierarchy, its use may be a symptom that the Open/Closed and/or Single responsibility principles are being violated, or that responsibilities may otherwise be misaligned.  This is not to say that every case of Double Dispatch means something is amiss, but only that its use should be a flag to reconsider your design in light of future maintenance needs.

About Derek Greer

Derek Greer is a consultant, aspiring software craftsman and agile enthusiast currently specializing in C# development on the .Net platform.
This entry was posted in Uncategorized and tagged . Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://www.twitter.com/justmikesmith Michael A. Smith

    Excellently observed Derek. Thanks for sharing.

  • http://www.derekhammer.com Derek Hammer

    Double Dispatch is certainly a sharp tool and you point out a way that you can get hurt by it.

    However, double dispatch is good if you have a relatively static structure (such as an abstract syntax tree; static here meaning static in the way that it is represented not the flexibility of the instances).

    Let us suppose that in your final example we were to create an IShape called Circle. It would be very tough to fit it into the the way that you draw your other shapes.

    However, in the double dispatch example, it would be trivially easy.

    If we can assume that the surfaces are constant (or nearly enough so), then double dispatch is a very effective tool and promotes good design (stability in the parts that don’t change and flexibility in the parts that do change).

    A great example of this is (as I previously mentioned) ASTs. Tree structures are pretty must how you’re always going to represent an AST. So, double-dispatch is a wonderful way to follow the Hollywood Principle as you walk an AST. This is used extensively in compilers :)

    Good point, though, mentioning that double dispatch isn’t always the solution!

  • http://realfiction.net Frank Quednau

    It’s kind of interesting that you didn’t mention “Visitor pattern” once. In that light I find the dispatch direction canvas -> shape a bit strange. Considering that a surface “visits” a number of shapes being in a polymorhic relationship, the drawing of the shape should be delegated to the canvas. Each canvas will know all shapes you care about while the shapes have no clue about the function of the visiting part. A canvas may know how shapes come to life in its context and contain the corresponding code.

  • http://ashmind@gmail.com Andrey Shchekin

    For some cases (such as mapping Expression Tree to SQL), I do not see another choice except using a visitor/multi-dispatch.

  • http://www.lostechies.com/members/derekgreer/default.aspx derekgreer

    Derek H,

    Yes, I can see how a static hierarchy of objects like ASTs wouldn’t pose the same maintenance/extensibility concerns.

    As far as any type of ellipse goes, you’re right that my design doesn’t account for that and the line segment approach is certainly a naive one. That said, I don’t think I’d turn to double dispatch as a way of introducing flexibility to such a design.

    Thanks for the feedback.

  • http://www.lostechies.com/members/derekgreer/default.aspx derekgreer

    Frank,

    Actually, I was planning on discussing Visitor as the subject of my next post. Double dispatch is often found in Visitor, but I wanted to keep this article a bit more focused.

    As far as the dispatch direction goes, either way seems a bit arbitrary to me. I can draw any shape I want on a piece of paper, so having a surface know what shapes can be drawn seems equally wrong aside from the extensibility and maintenance concerns. The purpose was really to convey the concepts, not to highlight my modeling skills :) If it helps, make surface an asteroid and shapes a spaceship.

  • http://www.lostechies.com/members/derick.bailey/default.aspx derick.bailey

    great writeup, derek. glad you took the time post it.

    i can see now from your “polymorphic static binding” section why double dispatch is so commonly confused with a strategy pattern implementation… that little bit of syntax seems to be what most people (myself included) focused on as the double dispatch.

  • http://nolanegly.edgewolf.com Nolan Egly

    Derek,

    Great write up. Based on your article and the one at Wikipedia, double disptach appears to be a workaround for the limitations of inheritence doing the expected thing in a statically bound language.

    While I like type safety, double dispatch (write the Draw method on a shape for every available surface) makes me wince; it quickly leads to combinatorial explosion. I think a lot of this would go away in a dynamically bound language.

    Another alternative to double dispatch would be the bridge pattern. Shapes receive an ISurface reference that defines methods like DrawLine(Point start, Point stop) and DrawArc(Point center, Point start, Point stop), etc.

    This allows each concern to vary more separately (how to draw on the surface for ISurface, and what to draw within Shape), doesn’t require adding a new Draw method for every new Surface like DoubleDispatch, and doesn’t necessarily mean changing existing Shape methods when new Surface methods are added (like DrawSpiral() or whatever).

    This is all just an example of course and a real life situation might make the Bridge pattern less attractive or Double Disptach more attractive for other reasons.

  • http://zvolkov.com/blog zvolkov

    IF statements are bad, let’s replace em with Double-Dispatch!
    Double-Dispatch is bad, let’s replace it with dictionary!
    The direction seems to be from imperative to declarative.

  • http://www.lostechies.com/members/derekgreer/default.aspx derekgreer

    @zvolkov I’m not sure I would consider this declarative, but that aside, I’d describe the direction as toward single responsibility. Replacing ‘if’ statements for testing runtime type information with double dispatch is one way of designing more focused methods, while what I’m advocating in this article is single responsibility at the class level.

  • nuttycom

    This general class of problems which is usually addressed using multiple dynamic dispatch, double static dispatch and /or pattern matching is usually referred to as the “expression problem.” The choice of whether to use double dispatch to solve it is entirely situational since there are always tradeoffs and it’s one of the cleanest ways available to recover subtype information. Of course some would argue that subtyping itself is the technique whose value should be questioned. :)

  • Mark

    I’m not sure I understand how your ‘polymorphic static binding’ example is not using double dispatch.

    It seems that shape.Draw(this); still does a dynamic lookup on the runtime type of shape, (using static types for the arguments). In your example, there is only one concrete Shape, so it resolves the same way but if you extended shape to add another strategy I think it will be obvious that it’s double dispatch.

    Personally, I don’t think Double Dispatch is a code smell. It’s a work around to a programming limitation that avoids long lists of ‘instanceof’.

  • http://www.lostechies.com/members/derekgreer/default.aspx derekgreer

    @nuttycom

    Double dispatch doesn’t seem to solve the expression problem as I understand it because the second level dispatch occurs on a class which has to be modified to add new cases.

    As far as being the cleanest approach goes, this estimation is subjective. Given adherence to the Open/Closed Principle isn’t generally considered, thereby requiring less constructs to arrive at the highest level of extensibility, I think we might be able to say it’s definitely simpler than other approaches, but simpler doesn’t necessarily mean cleaner.

    @Mark

    It’s not double dispatch because only one vtable lookup is performed by the runtime in this example. Double dispatch isn’t about calling parameter.Do(this) in the same way the Law of Demeter isn’t a dot counting exercise. You are right however that extending Shape makes it double dispatch. That is the difference between my polymorphic static binding example and my double dispatch example.

    As far as whether Double Dispatch is a code smell or not goes, this determination should be made based upon whether or not its a common indicator of violating good design principles. While there are some valid uses of Double Dispatch, the very nature of the technique tends to lead to a violation of the Open/Closed and Single Responsibility principles. Therefore, I believe it can rightly be considered a smell.