Should one make the database calls in the constructor or method of a class?

Question

Take the following instance for example: CreditCardApplication class

 public class CreditCardApplication
    {
        public int Id { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public int Age { get; set; }
        public decimal GrossAnnualIncome { get; set; }
        public string FrequentFlyerNumber { get; set; } 
    }

And there is a CreditCardApplicationEvaluator class

public class CreditCardApplicationEvaluator
    {
        private const int AutoReferralMaxAge = 20;
        private const int HighIncomeThreshhold = 100_000;
        private const int LowIncomeThreshhold = 20_000;

        public CreditCardApplicationDecision Evaluate(CreditCardApplication application)
        {
            if (application.GrossAnnualIncome >= HighIncomeThreshhold)
            {
                return CreditCardApplicationDecision.AutoAccepted;
            }

            if (application.Age <= AutoReferralMaxAge)
            {
                return CreditCardApplicationDecision.ReferredToHuman;
            }

            if (application.GrossAnnualIncome < LowIncomeThreshhold)
            {
                return CreditCardApplicationDecision.AutoDeclined;
            }

            return CreditCardApplicationDecision.ReferredToHuman;
        }       
    }

CreditCardApplicationDecision is an enum.

Now as you can see there are certain private const defined inside CreditCardApplicationEvaluator class. In a real world application these values will most probably come from a database.

So the question is where do I make the database call to set these constants? Inside the constructor? Or within Evaluate method?

Should I create another class Thresholds with the properties such as AutoReferralMaxAgeetc and pass it on to Evaluate method as follows:

static void Main(string[] args)
{
   //CreditCardApplication instance creation code goes here... 

   Thresholds thresholds =new Thresholds();
   thresholds = _unitOfWork.CreditcardRepository.GetThresholds();
   CreditCardApplicationEvaluator eval=new CreditCardApplicationEvaluator();
  var decision = eval.Evaluate(creditCardApplication,thresholds);
}

Or should I just place unitOfWork.CreditcardRepository.GetThresholds() inside Evaluate method of CreditCardApplicationEvaluator class, or perhaps it's constructor?

---

Calling database inside a constructor is probably a bad idea as pointed out by @Greg. So now what is left unanswered is that should I :

1. Have unitOfWork.CreditcardRepository.GetThresholds() inside main()
2. or within Evaluate() method of CreditCardApplicationEvaluator class

Answer 1

Constructors should not do work. The initialization of a new object should happen very quickly, and making database calls, or interacting with any resource outside of the current process, can take considerably longer.

Instead, your constructor should either require the data from the database as separate arguments, so that the database calls are made before creating the new object, or move that logic into an instance method.

Constructors should be limited to allocating new memory for the new object (like initializing empty collections or assigning default values to required fields) and ensuring the object is in a valid state upon first being created.

Anything beyond that belongs in an instance method, or arguments passed into the constructor.

To be specific to your use case, the three "const" integers should be arguments to the constructor. Furthermore, the income threshold is actually a good candidate for a new class, because you don't want a high threshold of 20,000 and a low threshold of 100,000.

public class IncomeThreshhold
{
    public int Low { get; }
    public int High { get; }

    public IncomeThreshhold(int low, int high)
    {
        if (low < 0)
            throw new ArgumentOutOfRangeException();

        if (high < 0)
            throw new ArgumentOutOfRangeException();

        if (high < low)
            throw new ArgumentOutOfRangeException();

        Low = low;
        High = high;
    }
}

Then your CreditCardApplicationEvaluator class becomes a little easier to construct with valid data:

public class CreditCardApplicationEvaluator
{
    private int AutoReferralMaxAge { get; }
    private IncomeThreshhold IncomeThreshhold { get; }

    public CreditCardApplicationEvaluator(int autoReferralMaxAge, IncomeThreshhold incomeThreshhold)
    {
        if (autoReferralMaxAge < someValueProbablyDrivenByRegulations)
            throw new ArgumentOutOfRangeException();

        AutoReferralMaxAge = autoReferralMaxAge;
        IncomeThreshhold = incomeThreshhold;
    }

    public CreditCardApplicationDecision Evaluate(CreditCardApplication application)
    {
        if (application.GrossAnnualIncome >= IncomeThreshhold.High)
        {
            return CreditCardApplicationDecision.AutoAccepted;
        }

        if (application.Age <= AutoReferralMaxAge)
        {
            return CreditCardApplicationDecision.ReferredToHuman;
        }

        if (application.GrossAnnualIncome < IncomeThreshhold.Low)
        {
            return CreditCardApplicationDecision.AutoDeclined;
        }

        return CreditCardApplicationDecision.ReferredToHuman;
    }
}

The initialization of these objects in conjunction with a unit of work or repository ends up like this:

static void Main(string[] args)
{
    var incomeThreshhold = unitOfWork.CreditcardRepository.GetThreshold(...);
    var autoReferralMaxAge = unitOfWork.CreditcardRepository.GetAutoReferralMaxAge(...);
    var evaluator = new CreditCardApplicationEvaluator(autoReferralMaxAge, incomeThreshhold);
    var application  unitOfWork.CreditcardRepository.GetApplication(...);
    var decision = evaluator.Evaluate(application);
}

Answer 2

When you instantiate the object CreditCardApplicationEvaluator, it needs to be ready to evaluate applications. We must think of the failure case where you can't pull this data from the database. So if you put the db call for those constants in the constructor, it might never return, so you will never instantiate your object or it will fail.

Also another reason to not put in the constructor is every single instantiation you are hitting the database. I assume the data is not updated in the db this frequently?

Also your db calls should be on the periphery and not tightly coupled to the business logic. What if I want to change where I get these constants from? Maybe I eventually want some from one data source and some from another? The evaluator class shouldn't be dependent upon some database.