CODEWITHSUNDEEP
c#classoopobjectconstructor
Jay
Jay

Reputation: 59

can I initialize array instance variable outside class? What is the best OOP approach to below code?

I have to implement switchboard class which can have devices like Fan, AC, Bulb etc. My switch board class looks like below.

Which one is more object oriented?

1.

class SwitchBoard
{
    public static int totalDevices=0;
    public List<ElectricDevice> Devices { get; set; }
    public SwitchBoard(int noOfFans, int noOfACs, int noOfBulbs)
    {
        Devices = new List<ElectricDevice>();
        //int deviceCount = 0;
        for (int i = 0; i < noOfACs + noOfBulbs + noOfFans; i++)
        {
            if (i < noOfFans)
            {
                Devices.Add(new Fan("Fan " + (i + 1),totalDevices));
            }

            else if (i >= noOfFans && i < noOfFans + noOfACs)
            {
                Devices.Add(new AC("AC " + (i - noOfFans + 1), totalDevices));
            }

            else
            {
                Devices.Add(new Bulb("Bulb " + (i - noOfFans - noOfACs + 1), totalDevices));
            }
            totalDevices++;
        }
    }
}

2.

class SwitchBoard
{
    public static int totalDevices=0;
    public List<ElectricDevice> Devices { get; set; }
    public SwitchBoard(int noOfFans, int noOfACs, int noOfBulbs)
    {
        Devices = new List<ElectricDevice>();

                CreateDevice(Devices, "Fan", noOfFans);
                CreateDevice(Devices, "AC", noOfACs);
                CreateDevice(Devices, "Bulb", noOfBulbs);

    }

I am feeling like first one is best approach because in second method we are using method which intstantiate the class outside class by taking property outside the class and intializing outside it. Like taking out switch out of the swtichboard and connecting it to fan and placing it back into switch board.

I think it has something to do with encapsulation.

Pseudo code for CreateDevice

function CreateDevice(List<EelectricDevice> Devices, string type, int noOfObjects )
{
for(int i=Devices.Length; i<noOfDevices+Devices.Length;i++)
{
         if(type=="Fan")
                 Device[i]=new Fan("Fan"+i);
         else if(type=="AC")
                 Device[i]=new AC("AC"+i);
      else if(type=="Bulb")
                 Device[i]=new Bulb("Bulb"+i);
}
}

Upvotes: 0

Views: 96

Answers (2)

Yogi
Yogi

Reputation: 9739

Designing of a system is something that is difficult to suggest as there are many things that needs to be considered.

But keeping it minimum and sticking to your question, there are a few things that needs to be considered -

Suggest to have an interface like ISwitchable, to be implemented by ElectricDevice which are switchable. This is important conceptually, as not all ElectricDevices are meant to be switchable - 

public interface ISwitchable
{
}

Now you can implement this in you device something like this - 

public class Fan : ElectricDevice, ISwitchable
{
    //Implementation of fan
}

For the total number of devices, it should be implemented as a read only property, which is calculated as a sum of all the devices, other there is a possibility of this value get tampered. There is no need to calculate this value with each iteration.

At the end this is how you SwitchBoard class might look like - 

public class SwitchBoard
{
    public List<ISwitchable> switchableDevices { get; private set; }

    public int TotalSwichableDevices
    {
        get { return numberOfFans + numberOfACs + numberOfBulbs; }
    }
    private readonly int numberOfFans;
    private readonly int numberOfACs;
    private readonly int numberOfBulbs;

    public SwitchBoard(int noOfFans, int noOfACs, int noOfBulbs)
    {
        this.numberOfFans = noOfFans;
        this.numberOfACs = noOfACs;
        this.numberOfBulbs = noOfBulbs;

        switchableDevices = new List<ISwitchable>();
        switchableDevices.AddRange(this.AddFans());

        //TODO: Add other devices
    }

    private IEnumerable<ISwitchable> AddFans()
    {
        List<ISwitchable> fans = new List<ISwitchable>();
        for (int i = 0; i < numberOfFans; i++)
        {
            fans.Add(new Fan());
        }
        return fans;
    }
}

Upvotes: 0

Sorin
Sorin

Reputation: 61

I would refactor that loop into a generic method to which you send a number (which represent the number of devices, and pass it function which returns an Electrical device:

public delegate ElectricDevice Func<ElectricDevice>()
private addElectricalDevices(int number, Func<int, ElectricDevice> createDeviceFunc)
{
   for(int i = 0; i < number; i++)
   {
      Devices.Add(createDeviceFunc(i));
   }
}

Example usage:

addElectricalDevices(10, (i) => new Fan("Fan " + (i + 1)));

Sorry, it's been long since i wrote C# code so this might not be perfect.

Upvotes: 1

Related Questions