Hello Dear DaniWeber's. I've been away for some time, and have recently made the transition from VB to C# (Finally). Using C# has opened my eyes to many things that I had either not fully understood, or had a lazy attitude towards. I find myself visiting the core of OOP and indeed polymorhism. Previousley I had used inheriting and implementing very loosley, maybe not even fully understanding their concepts and am now visiting each area and making sure I can implement things such as Interfaces and Abstract classes properly. In short, I'm trying to adopt a more professional approach to my development.

Below is (What I think) is a fair useage of interfaces, however, I am looking for advice for the community to guide me further. Is the code below acceptable, or even correct. My main focus of this question is the use of objects within Interfaces, is this common practice, or even acceptable? I have used the classic Vehicle example in order to try and depict what i have been doing.

I can create a new "Car" like so, and set some of it's properties:

Car MyCar = new Car();

MyCar.Wheels.WheelDiameter = 100;
MyCar.Wheels.WheelCount = 4;
MyCar.Wheels.NutsPerWheel = 1;

MyCar.Wheels.TyrePressure.FrontWheel = 36;
MyCar.Wheels.TyrePressure.RearWheel = 34;

The Interfaces and structure are as follows:

    /// <summary>Tyre Pressure Interface</summary>
    interface ITyrePressure
    {
        int FrontWheel { get; set; }

        int RearWheel { get; set; }
    }

    /// <summary>Tyre Pressure Object Implements ITyrePressure</summary>
    public class TyrePressure : ITyrePressure
    {
        int frontWheel;
        public int FrontWheel
        {
            get { return frontWheel; }
            set { frontWheel = value; }
        }

        int rearWheel;
        public int RearWheel
        {
            get { return rearWheel; }
            set { rearWheel = value; }
        }
    }

    /// <summary>Vehicle Wheels Interface</summary>
    interface IWheel
    {

        /// <summary>Vehicle Tyre Pressure Information</summary>
        TyrePressure TyrePressure { get; set; }

        /// <summary>Number of Wheels For Derived Vehicle</summary>
        int WheelCount { get; set; }

        /// <summary>Number of Nuts Per Wheel</summary>
        int NutsPerWheel { get; set; }

        /// <summary>Wheel Diameter</summary>
        int WheelDiameter { get; set; }

    }

    /// <summary>Vehicle Wheels Object, Implements IWheel</summary>
    public class Wheel : IWheel
    {

        //Initiate Child Objects
        public Wheel()
        {
            //Create A New Instance of The Tyre Pressure Class
            TyrePressure = new TyrePressure();
        }

        TyrePressure tyrePressure;
        public TyrePressure TyrePressure
        {
            get { return tyrePressure; }
            set { tyrePressure = value; }
        }

        int wheelCount;
        public int WheelCount
        {
            get { return wheelCount; }
            set { wheelCount = value; }
        }

        int nutsPerWeel;
        public int NutsPerWheel
        {
            get { return nutsPerWeel; }
            set { nutsPerWeel = value; }
        }

        int wheelDiameter;
        public int WheelDiameter
        {
            get { return wheelDiameter; }
            set { wheelDiameter = value; }
        }
    }

    /// <summary>Vehicle Interface</summary>
    interface IVehicle
    {

        /// <summary>Vehicle Wheel Data</summary>
        Wheel Wheels { get; set; }

        /// <summary>Get Current Fuel LEvel</summary>
        int GetFuelLevel { get; }

        /// <summary>Get Vehicles Current Speed</summary>
        int Speed { get; }

        /// <summary>Apply Brake Routine</summary>
        void ApplyBrake();

        /// <summary>Accelerate Routine</summary>
        void Accelerate();

        /// <summary>Change Gear Routine</summary>
        void ChangeGear();

        /// <summary>Event Hnadlers</summary>
        event EventHandler<int> MaximumSpeed;
        event EventHandler Stopped;
        event EventHandler<int> FuelWarning;
    }

    /// <summary>Car Class Implements IVehicle</summary>
    class Car : IVehicle
    {

        /// <summary>Initiat A New Car</summary>
        public Car()
        {
            fuel = 100;
            wheels = new Wheel();
        }

        /// <summary>Define Event Handlers</summary>
        public event EventHandler<int> MaximumSpeed;
        public event EventHandler Stopped;
        public event EventHandler<int> FuelWarning;

        int fuel;
        /// <summary>Get Remaining Fuel Level</summary>
        public int GetFuelLevel { get { return fuel; } }

        int speed;
        /// <summary>Get Car's Current Speed</summary>
        public int Speed { get { return speed; } }

        /// <summary>Apply Brake</summary>
        public void ApplyBrake()
        {

            Console.WriteLine("Screeeeeech!");

            speed -= 10;

            //If the speed is, or is below 0
            if (Speed <= 0)
            {
                //Correct any mistaken negative values.
                speed = 0;

                EventHandler handler = Stopped;

                if (handler != null)
                {
                    //Inform the driver they have stopped.
                    handler(this, null);
                }
            }

        }

        /// <summary>Accelerate</summary>
        public void Accelerate()
        {

            EventHandler<int> handler = MaximumSpeed;

            //Check to see if the car has reached it's speed limit.
            if (Speed >= 100 && handler != null)
            {
                //Alert the driver the maximum speed has been reached.
                handler(this, Speed);
                return;
            }

            //Increase the car's speed.
            speed += 10;
            fuel -= 1;

            Console.WriteLine("Vaarooooooooom!");

        }

        /// <summary>Change Gear</summary>
        public void ChangeGear()
        {
            //Not used to a clutch...
            Console.WriteLine("Crunch, Clunk, Grrrrrrrr");
            fuel -= 1;
        }


        Wheel wheels;
        /// <summary>Car Wheel Data</summary>
        public Wheel Wheels
        {
            get { return wheels; }
            set { wheels = value; }

        }


    }

Any advice or suggestions would be much apreciated.

Edited 1 Year Ago by J.C. SolvoTerra

My main focus of this question is the use of objects within Interfaces, is this common practice, or even acceptable?

Your interfaces contain properties rather than objects, which is just fine and quite common.

My issue with the code is twofold:

  1. It's overly complicated for what seems to be no benefit.

  2. The interfaces appear to be there for no reason other than to use interfaces.

So you want a car class, right? Very cool. What would I do differently? Just about everything. ;) Let's go top down.

interface ITyrePressure
{
    int FrontWheel { get; set; }
    int RearWheel { get; set; }
}

An object representing pressure makes marginal sense, but the logic is inverted here. A tire has pressure, but the pressure shouldn't give two hoots about about the tire. If you're going to dig this deeply into the class structure, I'd do it more like this:

class TyrePressure
{
    private int Lo { get; set; }
    private int Hi { get; set; }
    private int Current { get; set; }

    ...
}

class Tyre
{
    private TyrePressure Pressure { get; set; }

    ...
}

The idea being some programmatic way of telling the driver that the tire pressure is too low or too high for a single tire as that's really the only thing that makes sense to me if you're going down to tire pressure granularity.

The wheel class is pointless, but yours also exhibits an inversion of data in that a wheel doesn't know or care about other wheels, so WheelCount is in the wrong class. It's also unnecessary, but we'll get to that in a moment.

The reason I say the class is pointless, is because when all of the properties are non-moving, unmeasured physical components (ie. lug nuts and diameter), you can really just store the pressure class in your car and link it to known wheels. That's really all your car class needs to worry about from a software perspective. Once again I'm viewing this from a realistic vehicle computer situation: what does the vehicle's computer care about?

IVehicle is better designed since it has good useful stuff in it. But I'm still not seeing the benefit of the interface here since you probably want some kind of default behavior that's better suited to a base class. The Car class can be a Vehicle class and cover all of your bases without over-engineering things:

abstract class Vehicle
{
    public List<TyrePressure> Wheels { get; private set; }

    public int FuelMax { get; private set; }
    public int FuelCurrent { get; set; }

    public int SpeedMax { get; private set; }
    public int SpeedCurrent { get; private set; }

    ...
}

This class can viably be an abstract base because a concrete 'vehicle' is meaningless. But the base class would contain everything that's consistent between all vehicles you plan to support as well as default behavior.

So that's my off the cuff professional opinion. Most likely something like that would be a starting point with little to no other information and it would evolve into a more solid design. Also keep in mind that this post is quite literally a stream of consciousness, so I could have missed something or made a glaring mistake. ;)

Comments
Great advice
Glaring mistake or not, great answer!

Great Deceptikon, as per usual (How have you been btw).

I am going to spend much more time discecting your detailed response for future referance when designing a structure.

The important part, which I didn't make too clear is that the general implementation of objects (which are properties, thanks) and interfaces is acceptable... though in this scenario, very unnessasary and too complex, as you quite rightly point out and actually incorrectly structured lol.

I think my primary goal, well I say think... My primary goal was to try and ensure properties could be accessed MyCar.Wheels.TyrePressure.FrontWheel like so. I hate huge lists of properties (itellisense) and really enjoy trying to categorize properties in a much more structured way.

That being said, since the move to C#, I have noticed that attention to the finer details has a much more defined point than in VB. As with your answer, I need to pay special attention to various things like over complicating scenarios, the correct location of data within a structure etc.

Thank you, your response has certainly given me more pointers and diretion.

final note (So I don't seem like a complete melon), indeed some\most of the interfaces purely exist to be implemented, there is no actual requirement for them to be used other then to "just use them" and practice using them. The Whole structure of interfaces is more... How to describe it... representing data what you might find in a cars manual regarding what a tyres pressure should be, what the original wheel diameters are etc, e.g. lets say I was building a schema for MongoDB to hold data regarding types of vehicle etc.

Edited 1 Year Ago by J.C. SolvoTerra

I also just noticed this: public List<TyrePressure> Wheels { get; private set; }

Does that do what I think it does, allows the local member to be set internally but not externally... so as it suggests, readonly externally, read\write internally?

With this

public List<TyrePressure> Wheels { get; private set; }

I think you can still change Items in the list. Often you see this kind of thing if you don't want things to be changed:

public IEnumerable<TyrePressure> Wheels { get; private set; }

I think you can still change Items in the list.

You can. Though it's situational whether changing the type to IEnumerable<> is appropriate as it's not a comparable solution. My preference is for truly read-only properties to use ReadOnlyCollection<> or IReadOnlyList<> as it's very clear what the intention is. And for properties that don't need to be strictly read-only, IList<> or straight up List<> since the intention is still clear.

That said, 9 times out of 10 when I make a read-only list property, I change it to read-write down the road because it's unnecessarily restrictive most of the time.

How on earth have I not come across ReadOnlyCollection<> before?

Every day's a school day.

ReadOnly... Always made me laugh, you can't assign a new X to it, but you can manipulate all the content of X... Wow!

This article has been dead for over six months. Start a new discussion instead.