C# OOP inheritance class structure

Question

3.67/5 (2 votes)

See more:

hi all,

I am developing application which would be abble to supervise some servers inside network as i am learning OOP to be fully correct way i implemented below code. The target is from Program class i will call Heartbeatsupervisor class (engine class) that would take 2 parameters inside construcot of Server class and Protocol class and then do some work in StartCheck method inside Heartbeatsupervisor as well. Could you please be so kind check and give me your feedback if this is correct way created or something could be changed in case OOP. Whats your opinion? I tried to deal with more protected properties but i lose due to access. Firstly please check and give your opinion. Regards.

Base class Cocpit:

public abstract class Cocpit: ICheck
    {

        protected string _id;
        protected string _name;

        public Cocpit()
        {
        }

        public Cocpit(string name)
        {
            Console.WriteLine("Default constructor of Cocpit called");
        }

        protected internal abstract string Id { get; set; }
        protected internal abstract string Name { get; set; }

        #region ICheck Members

        public virtual void StartCheck()
        {
            Console.WriteLine("Start check");
        }

        public virtual void StopCheck()
        {
            Console.WriteLine("Stop Check");
        }

        public virtual Dictionary<string, string> GetDictionary()
        {   
            return null;
        }

        #endregion

    }

Server class derived from Cocpit:

public class Server: Cocpit
    {

        private string _ip;             //new field
        private Dictionary<string, string> _serverList;

        public Server(string name, string ip)
            : base(name)
        {
            this._ip = ip;
            this.Name = name;
            Console.WriteLine("Server default constructor called");
        }

        protected internal override string Id      
        {
            get { return _id; }
            set { _id = value; }
        }        //overriding property

        protected internal override string Name    
        {
            get { return _name; }
            set { _name = value; }
        }        //overriding property

        protected internal virtual string Ip               
        {
            get { return _ip; }
            set { _ip = value; }
        }        //new property

        #region Methods
        public Dictionary<string, string> ServerList
        {
            get { return _serverList; }
            private set { _serverList = value; }
        }

        #endregion

        #region Destructor
        
        /// <summary>
        /// Desctructor
        /// </summary>
        ~Server()
        {
        }

        #endregion

        #region Overrided Methods

        public override void StartCheck()
        {
            base.StartCheck();
            Console.WriteLine("StartCheck overrided");

            //TODO: [StartCheck] Check engine to be deployed
        }

        public override void StopCheck()
        {
            base.StopCheck();
            Console.WriteLine("StopCheck overrided");

            //TODO: [StopCheck] Check engine to be deployed
        }

Protocol class derived from Cocpit:

public class Protocol: Cocpit
  {

      /// This constructor will call Cocpit.Cocpit()
      public Protocol(string name)
          : base(name)
      {
      }

      protected internal override string Id
      {
          get { return _id;}
          set{_id = value;}
      }        //overriding property

      protected internal override string Name
      {
          get { return _name; }
          set { _name = value; }
      }        //overriding property

      #region Methods

      void EnumerateLists(IList list)
      {
          //notice that the list is passed an IList and hence can handle both types of lists
          //Use IEnumerable
          foreach (object val in list)
          {

          }
      }

      #endregion

  }

Heartbeatsupervisor class (my engine):

public class HeartbeatSupervisor : ICheck
{

    private string _serverIp;
    private string _protocolName;

    public string ServerIp
    {
        get { return _serverIp; }
        set { _serverIp = value; }
    }

    public string ProtocolName
    {
        get { return _protocolName; }
        set { _protocolName = value; }
    }

    public HeartbeatSupervisor(Server server, Protocol protocol)
    {
        this._serverIp = server.Ip;
        this._protocolName = server.Name;
    }

    #region ICheck Members

    public void StartCheck()
    {
        Ping pingSender = new Ping();
        PingOptions options = new PingOptions();

        // Use the default Ttl value which is 128,
        // but change the fragmentation behavior.
        options.DontFragment = true;

        // Create a buffer of 32 bytes of data to be transmitted.
        string data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
        byte[] buffer = Encoding.ASCII.GetBytes(data);
        int timeout = 120;

        PingReply reply = pingSender.Send(_serverIp, timeout, buffer, options);
        if (reply.Status == IPStatus.Success)
        {
            //Console.WriteLine("Address: {0}", reply.Address.ToString());
            //Console.WriteLine("RoundTrip time: {0}", reply.RoundtripTime);
            //Console.WriteLine("Time to live: {0}", reply.Options.Ttl);
            //Console.WriteLine("Don't fragment: {0}", reply.Options.DontFragment);
            //Console.WriteLine("Buffer size: {0}", reply.Buffer.Length);
        }
        else
        {
            //Dump to log file and send by email
        }

        StopCheck();
    }

    public void StopCheck()
    {
        //to implement
    }

    #endregion

}

and at last Program class to fire actions :) :

class Program
  {
      static void Main(string[] args)
      {
          HeartbeatSupervisor dd;
          dd = new HeartbeatSupervisor(new Server("someserver", "127.0.0.1"), new Protocol("tcp"));
          dd.StartCheck(); //inside here ill use values passed to constructor
          //for Server and Protocol and do let say ping

      }
  }

Posted 10-Mar-14 22:40pm

carlito_brigante

Updated 11-Mar-14 2:20am

v6

Add a Solution

Comments

Sergey Alexandrovich Kryukov 11-Mar-14 14:12pm

It does not make so much sense, unless you provide description of each class, explain the purpose of each and its role in the big picture. And so on...
Also, from the very beginning: it makes no sense without showing the interface ICheck. Your code sample has to be self-consistent, if you want us to examine it. (However, I can see it through region. Bad thing is: why implicit interface implementation, through public? Is it really needed? why?)
—SA

carlito_brigante 11-Mar-14 14:55pm

hi there,

the target of that app is to suervising some servers within a network.
The cocpit class is abstract base class for Protocol, Server and Service all of them has similar fields and i can say they have the same structure so i decided to make Cocpit as base class for them. Heartbeatsupervisor is lets say 'engine' which takes Server and Protocol within the constructor and will do e.g ping inside StartCheck method. Icheck is simple

namespace Stormin
{
interface ICheck
{
void StartCheck();
void StopCheck();
}
}

Sergey Alexandrovich Kryukov 11-Mar-14 15:05pm

You only need to make some class abstract, if you have some kind of polymorphism. So, what are the purposes of the terminal classes derived from Cocpit.
Having abstract Id and Name with protected _id and _name is not reasonable at all. You only open extra undesired access to members _id and _name without actual use of them.
—SA

carlito_brigante 12-Mar-14 4:07am

Could you tell me why this is not good in abstract? I declare property for Id which will take or set the _id assumed that is the way of OOP to access provate - this case protected field...

protected string _id;
protected internal abstract string Id { get; set; }

P.S the purpose of Cocpit i wrote eralier posts

Sergey Alexandrovich Kryukov 12-Mar-14 11:54am

Because you are no using it, just carry out to derived classes. Dead members...
—SA

carlito_brigante 12-Mar-14 17:17pm

you mean because they are two completely different members because my property has its own back field? That's what you want to say? I already found out that this auto property got backing field but other way its abstract. Could you explain here?

If I would like to control variable from beginning maybe then this way : to implement inside my abstract class field and property and then I can use _id and Id in derived classes or even override later in derived classes property if I would like to
view sourceprint?

protected string _id;
protected virtual string Id { get return _id; set _id = value; }

Or if from beginning I don't know if either all derived classes would like to control variable or only just 1 or 2 of them I can just leave only a property inside abstract class and override property if there is a need in some derived class to control value + create a field for it as protected also in derived class:
view sourceprint?

protected virtual string Id { get ; set; }

What about now :) ? Am I right know?

Sergey Alexandrovich Kryukov 12-Mar-14 17:40pm

Actually, yes. You have double back fields per property, and one of them is never used. You can override the property, but you could introduce alternative backing field in the overriding class, then it would makes some sense... I would simply remove _name and _id.
Probably, this is not the only problem, only something which caught my eye immediately...
—SA

carlito_brigante 12-Mar-14 17:52pm

So to be fully sure my thinking is really correct please tell yes or not to my two statements:

First case:
I don't know since beginning whether derived classes inheriting from abstract base class would need to control values or not so I would implement like below then if some derived class don't want to control value then don't need to override this but just use Id directly from abstract base class and that's it but if some derived class would like to control Id then need to override Id for it own and create private field _id as well

abstract class MyAbstractClass
{
protected virtual string Id { get; set;}
}

Does this what I wrote is correct thinking?

Second case:
assuming that I know all derived classes would need to control Id value then I need to implement over abstract base class like this and override Id for their own and not _id.

abstract class MyAbstractClass
{
protected string _id
protected virtual string Id { get return _id ; set _id = value;}
}

Does second way is correct way of thinking?

Thanks in advance.

Sergey Alexandrovich Kryukov 12-Mar-14 18:34pm

No, it's wrong. (And your code samples won't even compile. :-)
I answered to this, please see Solution 1.
—SA

carlito_brigante 12-Mar-14 19:03pm

i think so. After all I will use Properties in base class like:
protected virtual string Id { get; set;}

and then use them directly in derived classes or if need to ad additional qualification to setter/getter or both I will override

but one thing makes me thinking - that derived classes need to implemented their own private fields - I would like better to have in anyway inside base. How to do then?

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

carlito_brigante · Answer 1 · 2014-03-12T23:21:00

I did like this after all :

C#

public abstract class Cocpit: ICheck
   {
 protected string id;
        protected virtual string Id
        {
            get { return id; }
            set { id = value; }
        }

        protected string name;
        protected virtual string Name
        {
            get { return name; }
            set { name = value; }
        }

in Server I did: (no need to implement own condition just use values

C#

public class Server: Cocpit
   {
  public Server(string name, string ip)
            : base(name)
        {
            base.Id = ip;     //could be without base word
            base.Name = name;

        }

inside Protocol I implemented new conditions for Id

C#

public class Protocol: Cocpit
{

    /// This constructor will call Cocpit.Cocpit()
    public Protocol(string name)
        : base(name)
    {
        base.Name = "dd";
    }

 protected override string Id
        {

            get { return null; }

            set
            {
                //dont need to use base word
                if (base.id != value) return;
                //DoSomethingOnWrite(id, value);
                base.id = value;
            } //set Id

        }

and after all when want to use inside Heartbeate I cant access protocol.Name inside constructor - how to access that?

C#

 this._protocolName = protocol.Name;
}

C#

public class HeartbeatSupervisor : ICheck
 {

     private string _serverIp;
     private string _protocolName;

     protected string ServerIp
     {
         get { return _serverIp; }
         set { _serverIp = value; }
     }

     protected string ProtocolName
     {
         get { return _protocolName; }
         set { _protocolName = value; }
     }

     public HeartbeatSupervisor(Server server, Protocol protocol)
     {
        this._serverIp = server.Ip;
        this._protocolName = protocol.Name;
     }

Sergey Alexandrovich Kryukov · Answer 2 · 2014-03-12T12:33:00

carlito_brigante wrote:
So to be fully sure my thinking is really correct please tell yes or not to my two statements: First case: I don't know since beginning whether derived classes inheriting from abstract base class would need to control values or not so I would implement like below then if some derived class don't want to control value then don't need to override this but just use Id directly from abstract base class and that's it but if some derived class would like to control Id then need to override Id for it own and create private field _id as well…

Wrong.

Let's see why would you need a property (not field) at all. You need it only because of its setter, or getter, or both, to provide some side effect on reading or writing the property value. Why having auto-implemented properties (with just { get; set; } instead of "real" getter and setter? Just because you want to preserve such possibility in future.

Your first flaw in your consideration is: "I don't know since beginning whether derived classes inheriting from abstract base class would need to control values or not, so…". If you don't know that, it means that you should assume that the derived class will have to control it. Isn't it obvious?

The second big flaw is that you think that explicit backing fields can add anything at all. In fact, auto-implemented backing is strictly equivalent to similar explicit backing (not counting the subtle but practically non-considerable problem of the name of that backing field). Having auto-implemented backing does not limit the ability to add those side effects in any way.

Your code in the comment I referenced won't even compile and does not makes sense, so I'll write proper illustrative example:

C#

abstract class MyAbstractClass {
    protected virtual string Id { get; set; } // no need in explicit backing, ever
} //class MyAbstractClass

// this class does nothing useful because it adds no functionality,
// so it does not make any sense:
class MyDerivedClassWhichDoesNothingUseful : MyAbstractClass {
    protected string _id;
    protected override string Id { get { return _id; } set { _id = value; } }
} //class MyDerivedClassWhichDoesNothingUseful

// overriding Id only makes sense if you introduce some side effect to the setter
// or to the getter, or both:
class MyDerivedClassWhichCanMakeSomeSense : MyAbstractClass {
    protected string id; // (name fixed to meet the Microsoft naming conventions)
    protected override string Id {
        get {
            DoSomethingOnRead(id);
            return id;
        } //getId
        set {
            if (id == value) return;
            DoSomethingOnWrite(id, value);
            id = value;
        } //set Id
    } //Id
    void DoSomethingOnRead(string value) { /* ... */ }
    void DoSomethingOnWrite(string oldValue, string newValue) { /* ... */ }
} //class MyDerivedClassWhichCanMakeSomeSense

Is it clear now?

Let me tell you again: your original code is questionable is some other ways: the role of the interface (which is only needed when you use late binding, polymorphic methods receiving parameters of some interface compile-time, mixing interfaces with virtual method/property mechanisms (nothing wrong about it, but everything should have its purpose), and so on. I only caught what seemed obvious from the first glance.

—SA

C# OOP inheritance class structure

2 solutions

Solution 3

Solution 2

Add your solution here

Preview 0