Selecting a function to call based on a enum without if-else (or a switch)

Question

I have an interface and 3 function

public interface IDrawingObject
    {
        void Draw(Color c);
    }

    public class DrawTriangle : IDrawingObject
    {
        public void Draw(Color c)
        {
            //for demo purpose
            Console.WriteLine("Drawing Triangle with color " + c.Name );

        }
    }

    public class DrawCircle : IDrawingObject
    {
        public void Draw(Color c)
        {
            //for demo purpose
            Console.WriteLine("Drawing Circle with color " + c.Name);
        }
    }

    public class DrawRectangle : IDrawingObject
    {
        public void Draw(Color c)
        {
            //for demo purpose
            Console.WriteLine("Drawing Rectangle with color " + c.Name);
        }
    }

and this enum

  public enum Shapes
    {
        Circle,
        Rectangle,
        Triangle
    }

and here can be a lot more function(and enums)

I want to have static void Draw(Shapes s, Color c) that selects the right function to call based on this enum, and it looks to me that using if-else(or switch will bloat the code)

So i took another approach which is to use a IDictionary

   private static IDictionary<Shapes, Action<Color>> Mapper = new Dictionary<Shapes, Action<Color>>
{
    { Shapes.Circle, (Color c) => { IDrawingObject draw = new DrawTriangle(); draw.Draw(c);} },
    { Shapes.Rectangle, (Color c) => { IDrawingObject draw = new DrawRectangle(); draw.Draw(c); } },
    { Shapes.Triangle, (Color c) => { IDrawingObject draw = new DrawCircle(); draw.Draw(c); } }
};

and my function is

public static void Draw(Shapes s, Color c)
        {
            if (Mapper.ContainsKey(s))
            {
                Mapper[s](c);
            }
        }

but still, it looks to me that I'm still doing a lot of unscary copy and paste

Is there any better way to do it?

P.S

I have looked here, here

Answer 1

while I wouldn't recommend it, you can use reflection to create an instance of the class by the name. Something like this (untested):

var draw = (IDrawingObject)Activator.CreateInstance("AssemblyName", "Draw" + shape.ToString());
draw.Draw();

Answer 2

This code isn't really cleaner than a switch, unless the dictionary is used by multiple methods. The way the drawing objects and interfaces are used, all Draw methods could easily be static methods on one class.

Answering the exact question, one could use Dictionary.TryGetValue :

public static void Draw(Shapes s, Color c)
{
    if (Mapper.TryGetValue(s,out var act))
    {
        act(c);
    }
 }

All the Draw methods could change to static methods since they don't use any instance members:

private static IDictionary<Shapes, Action<Color>> Mapper = new Dictionary<Shapes, Action<Color>>
{
    [Shapes.Circle]= (Color c) => DrawTriangle.Draw(c),
    [Shapes.Rectangle]= (Color c) => DrawRectangle.Draw(c),
    [Shapes.Triangle]=(Color c) => DrawCircle.Draw(c)
};

If not :

private static IDictionary<Shapes, Action<Color>> Mapper = new Dictionary<Shapes, Action<Color>>
{
    [Shapes.Circle]    = DrawTriangle.Draw,
    [Shapes.Rectangle] = DrawRectangle.Draw,
    [Shapes.Triangle]  = DrawCircle.Draw
};

Update

BTW that syntax shows, there's something weird going on. Using types instead of an enum would have prevented drawing circles when a circle is requested

We'd need to know more about where Shapes comes from, or why interfaces are used to create a better implementation, and avoid using the wrong implementation

Just for fun by the way, one could use C# 8's switch expressions together with the interfaces :

var drawer= shapes switch 
            {
                Shapes.Circle   =>new DrawingTriangle(),
                Shapes.Rectangle=>new DrawingRectangle(),
                Shapes.Triangle =>new DrawingCircle(),
                _ => ???
            };
drawer.Draw(c);

Answer 3

Take a look at your Draw methods: do they actually change anything on any of the DrawTriangle, DrawRectangle, etc, classes?

If not, then you don't need to create a new instance every time you want to draw something. Instead, you can just store a single instance of DrawTriangle inside your Mapper dictionary:

private static Dictionary<Shapes, IDrawingObject> Mapper = new Dictionary<Shapes, IDrawingObject>()
{
    { Shapes.Circle, new DrawCircle() },
    { Shapes.Rectangle, new DrawRectangle() },
    { Shapes.Triangle, new DrawTriangle() },
};

Then you fetch the appropriate IDrawingObject for a given Shapes, and call its Draw method:

public static void Draw(Shapes s, Color c)
{
    if (Mapper.TryGetValue(s, out IDrawingObject drawingObject))
    {
        drawingObject.Draw(c);
    }
}

---

If for some reason you do need to create a new DrawTriangle very time you want to draw a triangle, you can instead put Func<IDrawingObject> delegates in your dictionary, but still call IDrawingObject.Draw in your static Draw method:

private static Dictionary<Shapes, Func<IDrawingObject>> Mapper = new Dictionary<Shapes, IDrawingObject>()
{
    { Shapes.Circle, () => new DrawCircle() },
    { Shapes.Rectangle, () => new DrawRectangle() },
    { Shapes.Triangle, () => new DrawTriangle() },
};

Then:

public static void Draw(Shapes s, Color c)
{
    if (Mapper.TryGetValue(s, out Func<IDrawingObject> drawingObjectFactory))
    {
        IDrawingObject drawingObject = drawingObjectFactory();
        drawingObject.Draw(c);
    }
}

Answer 4

This is virtually what you have!

you can make your nasty enum look like a "proper" object by using extension methods. (though its a mirage....exposed by having the exception in the MkShape method)

so the big objection to this is its not typesafe, if you add new enums and forget to update MkShape, your code will crash...a fucntional language like F#, Scala etc, would warn you that this was bad.

your dictionary does nothing except swap a bit of complexitiy for a tiny performance benefit of the switch statement...i.e. don't bother unless you've got hundreds of enum values, it may even cost you performance (for small n)

public enum Shapes
{
    Circle,
    Rectangle,
    Triangle
}

public interface IShape
{
    void Draw(Color c);
}

public static class Shape
{
    public static void ExampleClientCode()
    {
        var s = Shapes.Circle;
        // your enum looks like a "proper" object
        s.Draw(Color.AliceBlue);
    }

    public static IShape MkShape(this Shapes s)
    {
        switch (s)
        {
            case Shapes.Circle:
                return new Circle();
            case Shapes.Rectangle:
                return new Rectangle();
            case Shapes.Triangle:
                return new Triangle();
            default:
                throw new Exception("nasty enum means I can't have typesafe switch statement");
        }
    }

    public static void Draw(this Shapes s,Color c)
    {
        s.MkShape().Draw(c);
    }
}

public class Triangle : IShape
{
    public void Draw(Color c)
    {
        //for demo purpose
        Console.WriteLine("Drawing Triangle with color " + c.Name);

    }
}

public class Circle : IShape
{
    public void Draw(Color c)
    {
        //for demo purpose
        Console.WriteLine("Drawing Circle with color " + c.Name);
    }
}

public class Rectangle : IShape
{
    public void Draw(Color c)
    {
        //for demo purpose
        Console.WriteLine("Drawing Rectangle with color " + c.Name);
    }
}

P.S.

I suspect that it may be useful to store the colour in the shape, but if you are tied to the enum, then you're tied to just knowing the shape and nothing else