From the book Professional Enterprise .Net, which has 5 star rating on Amazon th

Question

From the book Professional Enterprise .Net, which has 5 star rating on Amazon that I am doubting after having a read through. Here is a Borrower class (In C# but it's pretty basic; anyone can understand it) that is part of a Mortgage application the authors built:

    public List<BrokenBusinessRule> GetBrokenRules()
    {
        List<BrokenBusinessRule> brokenRules = new List<BrokenBusinessRule>();

        if (Age < 18)
            brokenRules.Add(new BrokenBusinessRule("Age", "A borrower must be over 18 years of age"));

        if (String.IsNullOrEmpty(FirstName))
            brokenRules.Add(new BrokenBusinessRule("FirstName", "A borrower must have a first name"));

        if (String.IsNullOrEmpty(LastName))
            brokenRules.Add(new BrokenBusinessRule("LastName", "A borrower must have a last name"));

        if (CreditScore == null)
            brokenRules.Add(new BrokenBusinessRule("CreditScore", "A borrower must have a credit score"));
        else if (CreditScore.GetBrokenRules().Count > 0)
        {
            AddToBrokenRulesList(brokenRules, CreditScore.GetBrokenRules());
        }

        if (BankAccount == null)
            brokenRules.Add(new BrokenBusinessRule("BankAccount", "A borrower must have a bank account defined"));
        else if (BankAccount.GetBrokenRules().Count > 0)
        {
            AddToBrokenRulesList(brokenRules, BankAccount.GetBrokenRules());
        }
        // ... more rules here ...
        return brokenRules;
    }
Full code listing on snipt.org .

What is confusing me is that the book is supposed to be about professional enterprise design. Maybe I am a bit biased because the author confesses in chapter 1 that he didn't genuinely know what decoupling was, or what SOLID meant until the 8th year of his programming career (and I think he wrote the book in year 8.1)

I am no expert but I don't feel comfortable with:

Too many if else statements.

The class both serves as an entity and has validation. Isn't that a smelly design? (You might need to view the full class to get some context)

Maybe I am wrong, but I don't want to pick up bad practices from a book which is supposed to be teaching an enterprise design. The book is full of similar code snippets and it is really bugging me now. If it is bad design, how could you avoid using too many if else statements.

I obviously do not expect you to rewrite the class, just provide a general idea of what could be done.

Explanation / Answer

I'm a .NET developer, so I wrote this in notepad using C# syntax (which might be identical to JAVA).

public class BorrowerValidator
{
private readonly Borrower _borrower;

public BorrowerValidator(Borrower borrower)
{
_borrower = borrower;
Validate();
}

private readonly IList<ValidationResult> _validationResults;

public IList<ValidationResult> Results
{
get
{
return _validationResults;
}
}

private void Validate()
{
ValidateAge();
ValidateFirstName();
// Carry on
}

private void ValidateAge()
{
if (_borrower.Age < 18)
_validationResults.Add(new ValidationResult("Age", "Borrower must be over 18 years of age");
}

private void ValidateFirstname()
{
if (String.IsNUllOrEmpty(_borrower.Firstname))
_validationResults.Add(new ValidationResult("Firstname", "A borrower must have a first name");
}   
}

Usage:

var borrower = new Borrower
{
Age = 17,
Firstname = String.Empty
};

var validationResults = new BorrowerValidator(borrower).Results;
There is no point in passing borrower object to ValidateAge and ValidateFirstName, instead, just store it as a field.

To take things further, you can extract an interface: IBorrowerValidator. Say you then have different ways to validate your borrowers depending on their financial circumstances. For example, if somebody is ex-bankrupt, you may have:

public class ExBankruptBorrowerValidator : IBorrowerValidator
{
// Much stricter validation criteria
}

Related Questions

Navigate

• Browse (All)
• Browse F
• Subjects

Integrity-first tutoring: explanations and feedback only — we do not complete graded work. Learn more.