locked
Confusion about class design RRS feed

  • Question

  • I have an Employee object. One of the properties is Secretary. Another property is Supervisor. Both are Employee objects.  

    I instantiate an Employee who happens to be a secretary. As part of the process the constructor sets the Supervisor property by creating a new Employee object. We are now in a constructor for a new Employee object for the supervisor. As part of that process, it instantiates a new Employee object for that supervisor's secretary (who is the same person as above).  And so on. We are in an infinite loop. 

    What is the proper way to do this? Even if I have a Secretary class and a Supervisor class that inherit from Employee, I have the same problem. The Secretary creates a Supervisor, which creates a Secretary, which creates a Supervisor, etc. 

    Wednesday, December 17, 2014 12:59 AM

Answers

  • Greetings Maxx.

    The problem isn't that the secretary and supervisor have properties linking them together, the problem is constructing one within the other. Constructing instances of a class within the same class is asking for trouble.

    Also, logically it's not always the case that a supervisor has a secretary and a secretary has a supervisor. There might be a gap between the resignation of one or the other and the arrival of his/her replacement, for example.

    You need to instantiate the two employees separately, outside of the class, then link them together.

    Employee supervisor = new Employee();
    if(supervisor.needsASecretary) // Some test to see if a secretary is required and available.
    {
       Employee secretary = new Employee();
    
       supervisor.Secretary = secretary;
       secretary.Supervisor = supervisor;
    }

    • Proposed as answer by Arno Brouwer Wednesday, December 17, 2014 3:50 PM
    • Marked as answer by Maxx C Wednesday, December 17, 2014 3:54 PM
    Wednesday, December 17, 2014 2:30 AM

All replies

  • Hi,

    Just remove the Secretary and Supervisor in the Employee class; does not make any sense.

        public class Employee
        {
            public int EmployeeId { get; set; }
            public string Name { get; set; }
        }
    
        public class Secretary : Employee
        {
        }
    
        public class Supervisor : Employee
        {
        }
    
    
    // usage
    
      Employee emp1 = new Supervisor();
      Employee emp2 = new Secretary();
    
      Secretary sec1 = new Secretary();


    Please remember to mark the replies as answers if they help and unmark them if they provide no help.

    Wednesday, December 17, 2014 1:44 AM
  • Your description is a little bit confusing but what I understood is you have a Employee base class and there are Supervisor and Secretary sub-classes that inherit from Employee.

    // we can make this Employee class as an abstract class and in that case, Employee class would not get //instantiated and it will be pure base class for the classes that inherit from Employee.

    //But I am not making it an abstract class in this example

    public class Employee

    {

    // let say it has some properties such as:

    public int salary {get; set;}

    public void CalculateSalary()

    {

    this.salary = 1000;

    }

    }

    // Supervisor and Secretary classes created as subclasses of Employee

    public class Supervisor : Employee

    {

    // let say Supervisor has a property of coworker number

    public int numberOfCoWorkers {get; set;}

    }

    public class Secretary : Employee

    {

    // let say we are overriding the CalculateSalary method for Secretary

    public  override void CalculateSalary()

    {

    this.salary = 500;

    }

    }

    you can now instantiate each of these classes such as:

    Secretary secretary = new Secretary();

    secretary.CalculateSalary();

    Console.WriteLine(secretary.salary);

    // this will print 500 on the console log.

    Wednesday, December 17, 2014 1:50 AM
  • Thank you for the replies, but these seem to be taking me backward a step. If I create an Employee object for someone who is a secretary (or create a Secretary object), how do I know who that person's supervisor is, if there is no property that tells me? 
    Wednesday, December 17, 2014 2:11 AM
  • Greetings Maxx.

    The problem isn't that the secretary and supervisor have properties linking them together, the problem is constructing one within the other. Constructing instances of a class within the same class is asking for trouble.

    Also, logically it's not always the case that a supervisor has a secretary and a secretary has a supervisor. There might be a gap between the resignation of one or the other and the arrival of his/her replacement, for example.

    You need to instantiate the two employees separately, outside of the class, then link them together.

    Employee supervisor = new Employee();
    if(supervisor.needsASecretary) // Some test to see if a secretary is required and available.
    {
       Employee secretary = new Employee();
    
       supervisor.Secretary = secretary;
       secretary.Supervisor = supervisor;
    }

    • Proposed as answer by Arno Brouwer Wednesday, December 17, 2014 3:50 PM
    • Marked as answer by Maxx C Wednesday, December 17, 2014 3:54 PM
    Wednesday, December 17, 2014 2:30 AM
  • If you want to learn to design a class in a proper way then you can take a look of this:

    https://curah.microsoft.com/275639/design-patterns

    chanmm


    chanmm

    Wednesday, December 17, 2014 5:47 AM
  • Then get the object instead that you instance a new object for the secretary.


    Success
    Cor

    Wednesday, December 17, 2014 11:54 AM
  • Why don't you have a property in the Employee object that indicates the Employee's role? Employee.Type 1 = Supervisor and 2 = Secretary so on and so forth. Anything else you are trying to do is overkill here
    Wednesday, December 17, 2014 3:41 PM
  • Thank you, Ante Meridian, for the clear and concise answer. That makes sense. 
    Wednesday, December 17, 2014 3:56 PM