4
\$\begingroup\$

In a wage processing system which employees MVP pattern We have Model classes WageInfo, EarningInfo, DeductionInfo and AdvanceInfo and entity class Advance. (With several other classes)

EarningInfo and DeductionInfo has a composition relationship with WageInfo (contained in WageInfo) and also since Advance amount is a part of wages (paid in advance) I've modeled the relationship of Advance with DeductionInfo as aggregation because finally it becomes a deduction to the wages (advance amount should be deducted from wages).

public class DeductionInfo
{
    public int EmployeeID { get; set; }
    public string Name { get; set; }        
    public decimal LoanInstalmentAmount { get; set; }
    public decimal UniformInstalmentAmount { get; set; }
    public decimal InsuranceInstalmentAmount { get; set; }      
    public decimal AdvanceAmount { get { return Advance.Amount; } set { Advance.Amount = value; } }

    public Advance Advance = new Advance();
}


 public class Advance
{
    public int AdvanceID { get; set; }
    public decimal Amount { get; set; }
    public DateTime EnteredDate { get; set; }
}

So when I create a DeductionInfo object I simply assign a value to AdvanceAmount property and I'm not initializing other properties of Advance class in this instance.

My question are...

  1. Is the way I've modeled these class relationships logical and correct?
  2. Whether I've properly implemented aggregation in DeductionInfo?
  3. Since the advance amount is just a amount is it worth to use decimal type instead?
\$\endgroup\$
4
\$\begingroup\$

This question is very, very similar to another question you recently asked, so this answer will basically (hopefully!) clarify my other answer.

KISS: Keep It Simple, Stupid.

These classes represent database records. With Entity Framework you would define the relationships like this (I'd drop the Info suffix):

public class Deduction
{
    // primary key:
    public int Id { get; set; }

    public string Name { get; set; }        
    public decimal LoanInstalmentAmount { get; set; }
    public decimal UniformInstalmentAmount { get; set; }
    public decimal InsuranceInstalmentAmount { get; set; } 

    // foreign keys:
    public int AdvanceId { get; set; }
    public int EmployeeId { get; set; }

    // navigation properties:
    public virtual Advance Advance { get; set; }
    public virtual Employee Employee { get; set; }
}

public class Advance
{
    // primary key:
    public int Id { get; set; }

    public decimal Amount { get; set; }
    public DateTime EnteredDate { get; set; }

    // navigation property:
    public virtual ICollection<Deduction> Deductions { get; set; }
}

EF works with conventions - it determines that a property called Id is mapped to your primary key, and that a property called [AnotherEntityTypeName]Id is a foreign key property when there's a virtual navigation property of that other entity type - just with the types involved and the names of the properties, EF is "smart" enough to determine that there's a one-to-many relationship between Deduction and Advance. Now as @Malachi pointed out, the question is is this really what you want?

Only you can answer that. We review code, not database schemas.


Per your other question I see that you're not using Entity Framework. I don't see how that would impact the design of your POCO classes though; at the end of the day they're just classes that convey data between the database and your app, and if you're using ADO.NET then you have full control over how the classes get instantiated and loaded with data, so the classes you're using can be anything your sweet heart desires. Ok anything your business logic needs.


For example, the below code returns an IEnumerable<Deduction>, where Deduction is defined as above, eager-loading the related Advance when it exists:

public IEnumerable<Deduction> GetAllDeductions()
{
    var sql = "SELECT * FROM dbo.Deductions d LEFT JOIN dbo.Advance a ON d.AdvanceId = a.Id"
    using (var connection = new SqlConnection(_connectionString)
    {
        connection.Open();
        using (var command = new SqlCommand(sql, connection))
        using (var reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                var deduction = new Deduction();
                Advance advance;

                // assuming column names... this is just an example
                if (reader["EnteredDate"] != null)
                {
                    advance = new Advance()
                    advance.Id = reader["AdvanceId"];
                    advance.Amount = reader["Amount"];
                    advance.EnteredDate = reader["EnteredDate"];
                }

                deduction.Id = reader["Id"];
                deduction.Name = reader["Name"];
                deduction.LoanInstalmentAmount = reader["LoanInstalmentAmount"];
                deduction.UniformInstalmentAmount = reader["UniformInstalmentAmount"];
                deduction.InsuranceInstalmentAmount = reader["InsuranceInstalmentAmount"];

                deduction.Advance = advance;

                yield return deduction;
            }
        }
    }
}

This boilerplate-level gruntwork could all be automated with EF and look like this instead:

public IEnumerable<Deduction> GetAllDeductions()
{
    using (var context = new MyDbContext(_connectionString))
    {
        return context.Deductions.Select(e => e).Include(e => e.Advance).ToList();
    }
}

ViewModels

What you seem to want to display is different. It's a ViewModel, something that does not represent database records, but something meant to be displayed.

Then you create another class to represent just that. It's possible that such a class looks very similar to the entity types; it's also possible that it contains properties from two or more entity types:

public class DeductionInfo // or DeductionViewModel
{
    public int Id { get; set; } // only if the View needs it

    public string Name { get; set; }        
    public decimal LoanInstalmentAmount { get; set; }
    public decimal UniformInstalmentAmount { get; set; }
    public decimal InsuranceInstalmentAmount { get; set; } 
    public decimal AdvanceAmount { get; set; }
}

You'll have a service whose job is to get the data, prepare your ViewModels and spit out these DeductionInfo instances so that your View can display them.

Bottom line: the View doesn't care about the database schema, and doesn't need to know about your entity types. It works with a ViewModel that only contains the data that it needs.

An alternative could be to create a complex type (basically, the DeductionInfo class above), and to have the service fetch the data directly into that type, and return that type for the view to consume: you only select the data you need, and you consume it directly. Works just as well. Which is best? You decide!

\$\endgroup\$
3
\$\begingroup\$

Answers

  1. No. You have one class that needs another classes Properties for it's own Properties, Why aren't they the same Class/Object? take parameters from a constructor for filling in this information.

  2. Why Aggregate DeductionInfo as a Class? The Name DeductionInfo doesn't describe the parts well. By that I mean that the things inside this class don't say to me that this is Deduction information.

  3. Yes. That looks correct to me.


Clarification on Question #1

An Advance and a Deduction are two different things one adds and one subtracts, and Advance should go in Earnings and not Deductions. it is an advance of earnings, not a deduction from earnings.


Rants

Do you really want Advance to be public? Do you want just anything to change the values in that class? Is an Advance a Deduction?

It doesn't look like a good idea to me to have one classes properties be a reflection of another classes properties, why aren't they the same class?


I also don't think that you want to create a new Advance object inside of DeductionInfo all of the information will be blank or default values.


I don't see a reason why you would want your DeductionInfo setting your Advance.Amount it should not be set there.

public decimal AdvanceAmount 
{ 
    get {return Advance.Amount;} 
}

No Set.


\$\endgroup\$
  • \$\begingroup\$ So is it ok if I use a decimal type property in DeductionInfo instead of Advance object while having a separate class called advance? If its said that the relation ship is aggregation then how to model that in code please? Making advance public was a mistake really sorry for that. \$\endgroup\$ – CAD May 28 '14 at 16:11
  • \$\begingroup\$ @Chathuranga, merriam-webster.com/dictionary/aggregation yes you have a group of many distinct parts, whether they go together or not is a different story. \$\endgroup\$ – Malachi May 28 '14 at 16:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.