Create shapes in a console

I've done a job interview assignment on shapes (surprising ah?!) which is a C# console application which creates a list of shapes basically. It's available here, but for my question the only relevant part is the following:

This is where shapes are created - AddNewShape method (I presented this in the interview):

switch (pressedKey)
{
    case ConsoleKey.D1:
        shape = new Square(GetNumFromConsoleAndVerifyIt());
        break;
    case ConsoleKey.D2:
        double height = GetNumFromConsoleAndVerifyIt();
        double width = GetNumFromConsoleAndVerifyIt();
        shape = new Rectangle(height, width);
        break;
    case ConsoleKey.D3:
        shape = new Circle(GetNumFromConsoleAndVerifyIt());
        break;
    case ConsoleKey.D4:
        double triangleHeight = GetNumFromConsoleAndVerifyIt();
        double triangleWidth = GetNumFromConsoleAndVerifyIt();
        shape = new RightTriangle(triangleHeight, triangleWidth);
        break;
    default:
        throw new ArgumentException();
}

The problem that was discussed about this section is that it's not "Open for Extension" (Personally I prefer the 'Predicted Variations' version of this principle). So later on I refactored this part:

// Use reflection to get the type and it's required params:
Type shapeType = shapeTypes.Where(t => t.FullName.ToLower().Contains(shapeNameEntered.ToLower())).First();
var ctor = shapeType.GetConstructors()[0];

List sides = new List();
foreach (ParameterInfo pi in ctor.GetParameters())
{
    sides.Add(GetNumFromConsole(pi.Name));
}
shape = (I2DShape)ctor.Invoke(sides.ToArray());

And now you can clone my project and add whatever shape you want, implement I2DShape (also - have only one ctor) and change nothing in the Program.cs etc...

So, does this follow the principle? Did I get it right using reflection or should I have used a different approach?

I find neither approach is good.

What's wrong with the switch?

• It needs to be modified with each change and even only if you want to do something with only one shape.

• It's bound to key-codes.

• You cannot test it.

What's wrong with reflection?

• Reflection is usually the last resort solution. I think you are not at a dead end yet.

• The way you implemented it is very unsafe because you use the FullName and Contains. What if a Rectangle would be in a namespace like CirclesAndSqures and you are looking for a circle? You'll find something that is not a circle but a rectangle only because its namespace has the name circle in it.

• You cannot test it.

Instead I suggest the following. Let's say you have an abstract Shape

abstract class Shape { }

and some concrete types

class Circle : Shape { }

For each type you can write a factory that implements a common interface:

interface IShapeFactory
{
    Shape CreateShape();
}

Each factory of course can depend on other services like reading from the console etc.

class CircleFactory : IShapeFactory 
{
    public Shape CreateShape() 
    {
        // read params etc
        return new Circle();
    }
}

Finally you create a dictionary with all the shape factories

var shapeFactories = new Dictionary(StringComparer.OrdinalIgnoreCase)
{
    [nameof(Circle)] = new CircleFactory()
};

var shapeName = // read shape name...

if (shapeFactories.TryGetValue(shapeName, out IShapeFactory shapeFactory))
{
    var shape = shapeFactory.CreateShape();
}

You can now add as many shapes and factories as you want without touching any other shapes or common code.

StackExchange Code Review Q#157807, answer score: 9