Validate the set accessors in a simple Triangle class

Question

I have the following sample Triangle class and I want to validate the set accessors' values, obviously I cannot have a zero side or height Triangle. But while I am writing the validation rules for each property I found myself repeating the same code over and over again. Is there a more concise way to write these properties or to group them together? You can assume how newbie to C# I am.

    //  The Shape class
    abstract class Shape
    {
        // Shape's Methods
        abstract public double GetArea();
        abstract public double GetPerimeter();
    }

    //  The Triangle class
    class Triangle : Shape
    {
        private double sideA;
        private double sideB;
        private double baseT;
        private double height;

        public double SideA
        {
            get { return sideA; }
            set
            {
                if (value > 0)
                {
                    sideA = value;
                }
            }
        }

        public double SideB
        {
            get { return sideB; }
            set
            {
                if (value > 0)
                {
                    sideB = value;
                }
            }
        }

        public double BaseT
        {
            get { return baseT; }
            set
            {
                if (value > 0)
                {
                    baseT = value;
                }
            }
        }

        public double Height
        {
            get { return height; }
            set
            {
                if (value > 0)
                {
                    height = value;
                }
            }
        }

        // Triangle's constructor.
        public Triangle(double sideA, double sideB, double baseT, double height)
        {
            this.sideA = sideA;
            this.sideB = sideB;
            this.baseT = baseT;
            this.height = height;
        }

        // Get Triangle's area.
        public override double GetArea()
        {
            return (this.baseT * this.height) / 2;
        }

        // Get Triangle's perimeter.
        public override double GetPerimeter()
        {
            return sideA + sideB + baseT;
        }
    }

Answer 1

Assuming you'll have other shapes that would have the same requirement, one option you have is to have a class Side inside the class Shape, and put the validation in there. Now every shape can declare as many sides as needed and each will be validated. Some thing like this should work:

private class Side
{
    double length;
    public double Length
   {
        get{return length;}
        set
        {
            if(value > 0)
            {
                 length = value;
            }
            else
            {
                 throw new ArgumentException("Value must be greater than 0");
            }
         }
    }
    Side(double _length)
    {
        Length = _length;
    }
}

In regards to the height, instead of having it passed to the constructor, calculate the height from the length of the sides, Heron's formula should work.

Answer 2

There are a few problems with the code at the moment.

• As mentioned in the other answer, you can calculate height and area using just the side lengths, so there is no need to supply height as well.
• You are only doing validation in property setters, which means you can still create and use an invalid triangle using var triangle = new Triangle(0, 0, 0);. It might be a good idea to do validation in the constructor as well. This way your class will fail fast.
• When trying to set an invalid property, the call will silently fail, and the caller will have no idea anything is wrong. triangle.BaseT = 0; should throw a validation exception.
• The validation logic needs to be extended. What if you were to create a triangle with sides 1, 1, 3? Would that still be a valid triangle?
• Are you sure that you want to be able to change the sides of a triangle after it is created? Perhaps it would be easier to create a new triangle rather than modify an existing one? Removing the property setters would make the class immutable, which is often desirable, as it avoids surprises when sharing instances of the same class.

Assuming that you do indeed want a mutable triangle class, then you could extract validation into a method and re-use it from your constructor and property setters like so.

class Triangle : Shape
{
    private double sideA;
    private double sideB;
    private double baseT;

    public Triangle(double sideA, double sideB, double baseT)
    {
        SetSides(sideA, sideB, baseT);
    }

    public double SideA
    {
        get { return sideA; }
        set { SetSides(value, SideB, BaseT); }
    }

    public double SideB
    {
        get { return sideB; }
        set { SetSides(SideA, value, BaseT); }
    }

    public double BaseT
    {
        get { return baseT; }
        set { SetSides(SideA, SideB, value); }
    }

    private void SetSides(double sideA, double sideB, double baseT)
    {
        var sides = new[] {sideA, sideB, baseT};
        if (sides.Any(s => s <= 0))     // TODO: Perofrm extra triangle validation!
            throw new ArgumentException("Invalid triangle side lengths.");

        this.sideA = sideA;
        this.sideB = sideB;
        this.baseT = baseT;
    }
    ...
}