Class composition as instance variables

Question

I have a class called WageInfo and it has composition relationships with Earning and Deduction classes. So I implement this as follows...

class WageInfo
{
    int ID {get; set;}
    Earning E = new Earning();
    Deduction D = new Deduction();

}

I know that I havent initialized these object inside the WageInfo constructor so it is not possible to pass parameters to Earning and Deduction's constructors.

Other than that, can this be considered as composition? Any other flaws in this approach?

EDIT:

More details about classes

class Earning
{
int ID {get; set;}
Decimal Amount {get; set;}
DateTime Period {get; set;}

}

class Deduction
{
int ID {get; set;}
Decimal Amount {get; set;}
DateTime Period {get; set;}
String Remarks {get; set;}
}

And Im accessing these properties as follows...

In Program.cs

WageInfo WI= new WageInfo();
WI.E.Amount= 1000;

EDIT2:

My class relationships are like this...

enter image description here

WageInfo may have many Earnings, Deductions and WageBalances. So my PrepareEarnings() in WageManager class is as follows...

 public List<Earning> PrepareEarnings(DateTime wagePeriodStartDate, DateTime wagePeriodEndDate)
        {
            return GetEarningsToList(wagePeriodStartDate,wagePeriodEndDate);
        }


        public List<Earning> GetEarningsToList(DateTime startDate, DateTime endDate)
        {
            return CalculateEarnings(startDate,endDate).Rows.OfType<DataRow>().Select(CreateEarnings).ToList();
        }

        private DataTable CalculateEarnings(DateTime startDate, DateTime endDate)
        {
            return _DataService.GetEarnings(startDate,endDate);
        }

        private Earning CreateEarnings(DataRow row)
        {
            WageInfo wageInfo = new WageInfo();

            wageInfo.Earning.EmployeeID = row[0] == DBNull.Value ? 0 : Convert.ToInt32(row[0]);
            wageInfo.Earning.WorkDays = row[2] == DBNull.Value ? 0 : Convert.ToInt32(row[2]);

            //

            return wageInfo.Earning;
        }

I'm not sure if you have provided enough information to say if this is a good or a bad structure. From what I personally can see here it doesn't look bad... — drew_w, May 21 '14 at 13:09
Yes this can be considered composition. Regarding flaws: Its not necessarily bad practice, so you may go with this type of initialization. Also, you may pass parameters (e.g. anonymous types). Commenting because I am not sure if this is what you wanted to know ;-) — hschne, May 21 '14 at 13:18

score 2 · Answer 1 · edited May 23 '17 at 12:17

2

So, as far as I understand there is the underlying question of whether initializing variables outside of the constructor is valid in this case. Following this question I recommend the following:

Since WageInfo changes depending on Earnings and Deductions you should not use initialization outside of the constructor. Why not you may ask? Well, what happens if either Earnings or Deduction change?

WageInfo WI= new WageInfo();
WI.E.Amount= 1000;

This will get ugly very fast. However: If you do not make changes to these variables, your approach is valid. Generally speaking, I'd use this type of initialization whenever I do not plan on making changes to a variable, e.g. I could make it readonly.

In all other cases, I recommend using Dependecy Injection (or at least, a simple form of it). So, in your case, you could do

Earnings earnings = new Earnings(1000);
Deduction deduction = new Deduction(0);
WageInfo info = new WageInfo(earnings,deduction)

Edit: As mentioned in the comments below, it would be even better to hand responsiblity of construction over to the WageInfo class itself. One could add a method AddEarnings / AddDeductions where the corresponding objects are created.

WageInfo info = new WageInfo();
info.AddEarnings(100);

Also, like @Aleks mentioned, its a good idea to use private setters, at the very least. Do so by declaring properties like this:

public Earning { get; private set; }

edited May 23 '17 at 12:17

Community

1
1

answered May 21 '14 at 13:38

hschne

704
5
21

1

WageInfo info = new WageInfo(earnings,deduction) - This would not be aligned with the basic requirement for composition - lifecycle control. In this solution, E and D are created outside of the containing object and exist out of its context, like with a normal association. The correct way in this case would be to move the E and D construction inside the WageInfo, something like this WageInfo.addE(1000) -> now WegInfo controls the E itself, instead of an external entity. I agree that the WageInfo constructot is not the best choice for E and D creation, as they can be freely added – Aleks May 21 '14 at 14:22
@Aleks I am unfamiliar with the exact definition of composition. I was merely pointing out a different, and possibly more efficient approach. I absolutely agree with your proposal of moving the construction into WageInfo (e.g. WageInfo.AddE(...)). Thanks for pointing that out. – hschne May 21 '14 at 14:38
All cool, I've just pointed out what is important for a composition relationship, as Chathuranga told "this is a composition" and asked "if it can be considered as one" :) – Aleks May 21 '14 at 14:54
Actually in my `WageInfo` class I have only properties as its a Model class. All behaviors related to salary processing are in `WageManager class`. (like `CalcEarning()`, `CalcDeduction()` etc). So where to put this AddE(). I'll edit the post with a diagram. Thanks – CAD May 21 '14 at 17:27

score 1 · Accepted Answer · answered May 21 '14 at 13:25

An association is composition when a containing object (here WageInfo) fully controls the lifecycle of the contained object (E, D).

E and D are definitelly created in WageInfo, so it is ok for a composition. I suppose they will also be deleted from the WageInfo, so that's pretty enough to say that the relationship is composition.

In adition they should not be directly visible from outside of the containing object (private data members).

COmments to EDIT part:

You should make E and D private members in order to make the composition cleaner. At least a getter would do the job. This is not mandatory however, the lifecycle control is fundamental.

Which members should be private? Containing object's members (Private Earning E = new Earning()) or contained objects (Private Decimal Amount {get; set;})? — CAD, May 21 '14 at 13:29

Class composition as instance variables

2 Answers2