Is this breaking my SOLID Principle?

Question

I am trying to learn better programming practices using SOLID principles. Here I am working on a sample application of Shapes. I just want to know, am I breaking the principle anywhere.Below are classes and its code.

1. Base Class - Shape

public abstract class Shape
{
    public abstract double Area();
    public virtual double Volume() 
    { 
        throw new NotImplementedException("You cannot determine volume from here...Method not implemented."); 
    }
}

2. Classes for Shapes like Rectangle, Triangle etc implementing base class Shape.

public class Circle : Shape
{
    public int Radius { get; set; }
    public override double Area() { return 3.14 * Radius * Radius; }
}

public class Triangle : Shape
{
    public int Height { get; set; }
    public int Base { get; set; }
    public override double Area()
    {
        return 0.5 * Base * Height;
    }
}

public class Rectangle : Shape
{
   public int Length { get; set; }
   public int Breadth { get; set; }
   public override double Area()
   {
        return Length * Breadth;
   }
}

public class Square : Shape
{
    public Square() { }
    public int Side { get; set; }
    public override double Area()
    {
        return Side * Side;
    }
}

3. A factory class that returns Shape.

internal class ShapeFactory<K, T> where T : class, K, new()
{
    static K k;
    private ShapeFactory() { }

    public static K Create()
    {
        k = new T();
        return k;
    }
}

Till here everything seems fine and looks good, but problem occurs when I implemented it. I am little confused here. Lets see the front end code first:

internal class Program
{
    private static void Main(string[] args)
    {
        try
        {

            var c = ShapeFactory<Shape, Circle>.Create();
            // this part is not clear to me. See the questions below
            if(c is Circle)
            {
                var circle = c as Circle;
                circle.Radius = 5;
                Console.WriteLine(string.Format("{0}", circle.Area()));
            }


        }

        catch (Exception ex)
        {

            Console.WriteLine("Error: {0}", ex.Message);
        }
        Console.Read();
    }
}

QUESTIONS

Different shapes has got different properties like circle has Radius, triangle has base and height and so on , so i decided to keep my properties in child class. I knew, I can have that as virtual member in my base class. So Is there any way other than coded above.
If not, then what is the use of abstract class, if still I am typecasting my Shape object to circle object? I can simple use Circle c = new Circle(). I don't want unwanted checks like (if c is circle) and all.
What If , I am asked to implement a new method to get Circumference of a circle. Do I need to create a new Abstract class or put it in Circle class. But if I put it Circle, I think it will break very first principle of SOLID i.e. SRP . Kindly note, I don't my abstract class as a fat class having unnecessary or repeated properties.

Thanks in advance

codereview.stackexchange.com would be better suited for this question — Papa

guijob guijob · Accepted Answer · 2016-06-29T06:16:49

What I usually do in this case is to pass constructor parameters in concrete classes. So i'd change your concrete shapes to something like:

public class Circle : Shape
{
    public int Radius { get; set; }

    public Circle(int radius) {
        this.Radius = radius;
    }

    public override double Area() { return 3.14 * this.Radius * this.Radius; }
}

public class Rectangle : Shape
{
   public int Length { get; set; }
   public int Breadth { get; set; }

   public Rectangle(int lenght, int breadth) {
        this.Length = lenght;
        this.Breadth = breadth;
   }

   public override double Area()
   {
        return Length * Breadth;
   }
}

and so on

Now, I would use a factory method, so your fabric will now be like:

public abstract class ShapeFactory
{
    abstract Create();
}

public class CircleFactory : ShapeFactory
{
    private int radius;

    public CircleFactory(int radius){
        this.radius = radius;
    }

    protected override Shape Create()
    {
        return new Circle(this.radius);
    }
}

public class RectangleFactory : ShapeFactory
{
    private int length;
    private int breadth;

    public RectangleFactory(int length, int breadth){
        this.lenght = length;
        this.breadth = breadth;     
}

    protected override Shape Create()
    {
        return new Rectangle(this.length, this.breadth);
    }
}

Notice that, now a factory know how to build a shape with constructor passed in its own constructor.

So, each time you want a diferent shape you will instantiate a new factory.

ShapeFactory factory = new CircleFactory(5);
Shape shape = factory.Create();
Console.WriteLine(shape.Area()));

I think this answer your 1st and 2nd question.

So, 3: What you can do to dont modify your class is use the strategy pattern in order to pass at runtime how to implement this method:

public interface IPerimeter
{
    int calculatePerimeter();
}

public class Circunference : IPerimeter 
{
    public int calculatePerimeter(Circle circle) {
        return 2*pi*circle.radius;
    } 
}

public class Circle : Shape
{
    public int Radius { get; set; }
    private IPerimeter perimeter;

    public Circle(int radius, IPerimeter perimeter) {
        this.Radius = radius;
        this.perimeter = perimeter;
    }

    public Circunference() {
        perimeter.calculatePerimeter(this);
    }

    public override double Area() { return 3.14 * this.Radius * this.Radius; }
}

Hope this helps with your training.

Is this breaking my SOLID Principle?

4 Answers