Does this violate my SOLID principle?

Question

I am trying to learn the best programming techniques using SOLID principles. Here I am working on a sample Shapes application. I just want to know, I'm breaking the principle anywhere. The following are the classes and its code.

1. Base class - form

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, such as Rectangle, Triangle, etc., Implementing the 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. The factory class that returns the form.

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

So far, everything seems beautiful and looks good, but the problem arises when I implemented it. I'm a little confused here. First, consider the front end code:

 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 have different properties, such as a circle has a radius, a triangle has a base and height, etc., so I decided to save my properties in a child class. I knew I could have this as a virtual member in my base class. So there is some other way besides the coded above.

• If not, then what is the use of an abstract class, if nevertheless I attribute the Shape object to the environment object? I can just use Circle c = new Circle (). I don’t want unwanted checks like (if c is a circle) and that’s it.

• What if, I’m asked to introduce a new method to get the Circle Circle. I need to create a new abstract class or put it in the Circle class. But if I put Circle, I think that this violates the very first SOLID principle, i.e. SRP, Please note that I am not my abstract class as a bold class having unnecessary or duplicate properties.

Thanks in advance

Answer 1

What I usually do in this case is to pass constructor parameters in specific classes. Therefore, I would change your specific 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; } } 

etc.

Now I would use the factory method, so your fabric would look like this:

 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); } } 

Note that now the factory knows how to build a form with the constructor passed in its own constructor.

So, every time you need a different form, you create a new factory.

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

I think this is the answer to your first and second question.

So, 3: What you can do to not modify your class is to use a strategy template to pass at runtime a method for implementing 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 learning.

Answer 2

• Different child classes will have different properties, expected and approved. Usually, not all derived classes have the same properties as their base class. There is no reason to force Shape have Radius . What would be your advantage? It's just opening the door for trouble. What is your ultimate goal? Having something like myShape.Dimension = value and not caring if it's radius, side, etc.? Everything can be done depending on your needs.

• With your abstract class, you can, for example, skip the Shape list and call Area() or Volume() , knowing that you will get your result (despite the fact that you are not yet implemented by Volume ). Also your base class may have some common code that you are not using in this case. You could have, for example, the Unit property, which can be cm, inches, meters, etc., and then have a method like this (silly example):

   public string GetAreaString() { return string.Format("{0} {1}", this.Area().ToString(), this.Unit); } 

• Just implement it in Circle , of course. Why does this break Circle single responsibility? Your class does the calculations associated with this value, just as a string tells you whether it is zero or its length.

Answer 3

For me, your example seems really excessive. I think you should always implement the simplest thing that nothing else works. I know this is sample code because you want to learn SOLID principles, but I think it’s important to know how terribly mistaken these principles can be followed in the wrong context. In your specific code: do I need to group all the shapes using the Shape class? I mean, have you ever planned to sort out a list of shapes and calculate the area and volume for them? If not, inheritance makes absolutely no sense. Actually, I would say that inheritance is overused today, and when it is overused, you get ugly dependency dependency graphs. As for the factory class: Is the construction of any of your "curly" objects especially difficult, time-consuming, complicated. Does your factory class know any meaning or is it completely useless? If he has no real reason to exist, I would not use it; the new operator would be much clearer.

I hope you do not mind my answer, but I just wanted you to know that some SOLID principles apply in very specific scenarios. Forcing them to the wrong places can lead to ugly and complex code. In some situations in the real world, if yes is given to the questions above, your template looks OK. Otherwise, exactly the same picture can overly complicate the situation without any real advantages. I think my point is: just be careful, not every SOLID principle is good in any situation :).

Answer 4

This is a very common problem. While SOLID training is good, it requires an understanding of basic design principles such as abstraction and indirectness. The reason you get confused is because there is no abstraction in your code.

Imagine that you have code that wants to know the area of ​​the form, but it doesn’t care what the shape is and how to calculate this area of ​​the form. Something like:

 public void PrintArea(Shape shape) { Console.WriteLine(shape.Area()); } 

This is a CRITICAL PART of OOP. In your example, this is absolutely nothing. Your example is just a contrived piece of code that has no logic for it, not to mention that it is SOLID.