14

A few days ago I stumbled upon a silly "problem" that made me reflect about encapsulation and OOP design.

I have a class called User that has a method hasMinimumLegalAge() that checks if the user is legal (above 21 years) to perform some operations in the system:

class User {
    ...
    private static readonly MINIMUM_LEGAL_AGE: number = 21;
    private birthDate: Date

    ...

    hasMinimumLegalAge(currentDate: Date): boolean
    {
        //perform date difference between today - birthdate, check is user is above MINIMUM_LEGAL_AGE.
    }
}

In order for the method to be able to tell if the user is legal, it needs the current date to perform the date difference.

I have two options for this: hard-code the Date object inside the method or receive the current date as an argument.

Approach 1 (hard-code the Date object inside the method):

The problem with this approach is testability and coupling, I'll not have control over the date, thus i cannot easily test some scenarios, and my test will only be true for some time, but i gain "encapsulation" because know my method is able to respond to the message without any arguments (that seems ideal for me, in this case)

Approach 2 (receive the current date as an argument):

it's kind of weird to me to send a message "are you legal?" and then the method responds: "first I need to know what date is today", I feel like the object should have the necessary data to respond to this question, it feels like encapsulation is leaking because the method is depending on external information, even knowing that hard-coding the Date object is not ideal, but is it worth only for testability?

what are your thoughts?

12
  • 7
    Java has a Clock you can pass, so unit tests can pass their own fixed one. Better abstraction than a fake now.
    – Joop Eggen
    Apr 5 at 10:53
  • 2
    I think the question is not relative to Java but for OOP. So there should be a generic solution or approach for the problem. @JoopEggen
    – gavioto20
    Apr 5 at 10:59
  • 22
    The user is not the entity that decides what minimum legal age is. Probably best just to replace it with getBirthDate Apr 5 at 13:27
  • 11
    A little change might dissipate the problem: call your method hasMinimumLegalAgeOn(when: Date). It's obvious it takes a date. :-)
    – Pablo H
    Apr 5 at 14:21
  • 2
    In real life, who verifies a customer's age? Is it the customer itself or the bartender?
    – Tvde1
    Apr 6 at 8:23

11 Answers 11

57

Always pass the date as a parameter, otherwise you cant write tests.

From a modeling perspective, "current date" is something outside the user, shared by the whole system. If current date was encapsulated in the User object, then each user could have their own individual current date, which does not make sense.

Practically speaking, a function which tells if a User is of legal age on a given date is much more useful than than a function which can only tell if the user is legal today. Lets say the user put a reservation for a table at a bar on some date in the future. What matters is if the user is of legal age at the day of the bar visit, not if they are legal "today".

10
  • 3
    Some systems (such as Java 8 time) have a concept of a Clock that can be replaced with a test double. Apr 5 at 15:28
  • 14
    @BlueRaja-DannyPflughoeft test doubles are a workaround for a problem that should not be there in the first place. Passing the date is both simpler and conceptually more correct.
    – JacquesB
    Apr 5 at 18:33
  • 6
    The practical reason for passing the date is to ensure the date is consistent. Consider: you start an operation just before midnight, and the log routine generates its own just-before-midnight timestamp. The clock ticks over, and the "is legal age" check reads the clock, giving an answer of "yes". You then have a record of an apparent bug: it looks like "is legal age" returned "yes" one day too soon.
    – Mark
    Apr 5 at 23:26
  • 3
    Doesn't this just push the problem back to how we test the caller? Eventually you have to reach a function that uses the current date, and you'll need to mock the function that gets the date.
    – Barmar
    Apr 6 at 14:17
  • 3
    @Barmar At one point your test will call the top-level function which takes the Date as a parameter and there you can pass whatever date you want in.
    – Voo
    Apr 6 at 14:18
24

A pragmatic solution is to implement both:

hasCurrentlyMinimumLegalAge(): boolean
{
    return hasMinimumLegalAge(GetCurrentDate());
}

hasMinimumLegalAge(currentDate: Date): boolean
{
    //perform date difference between today - birthdate, check is user is above     MINIMUM_LEGAL_AGE.
}

That design provides a unit-testable method hasMinimumLegalAge. The method may be also used to check the legal age at another point in time than the current date, which can be perfectly sensible, not just for tests. It also provides a convenience method hasCurrentlyMinimumLegalAge which needs to be tested either manually or by an integration test, but which contains few enough code you may consider that as sufficient.

This is not a hypothetic approach, it is one you will find in several variants in lots of real-world applications.

(Note, as others have pointed out, for checking the legal age, it is not unlikely that the current date as a reference is rarely useful. But I can imagine cases where a reference to the current date & time is indeed the most frequent case.)

8
  • 1
    Would it not be better to have a default value for currentDate so both options can be covered by one function?
    – Drake P
    Apr 5 at 15:20
  • 3
    If your language supports those, then sure!
    – Kroltan
    Apr 5 at 16:33
  • @DrakeP: that's surely possible in many languages. If it is better is debatable, since it will be less expressive. I think it is more a matter of taste.
    – Doc Brown
    Apr 5 at 16:38
  • 2
    @DanielSchilling: such decisions depend on the context. If class User is part of a highly reusable framework provided by some vendor X, and used by a client Y, it is surely better to stick to the SRP "by the book". If it is exclusively used in a context where it can be changed by the team which uses it, pragmatic solutions are perfectly fine and can actually prevent overdesign. Moreover, your example is flawed: the member hasCurrentlyMinimumLegalAge() provides a single place where GetCurrentDate is called, so when the definition of a "current date" changes, ...
    – Doc Brown
    Apr 6 at 6:16
  • 1
    ... that's the one-and-only place to change. The real question one has to ask here: who maintains User, and who maintains changes to the definition of a current date? When both happens in the same team, my suggestion is fine. When it might happen in different team, it is better not to provide hasCurrentlyMinimumLegalAge in the User class.
    – Doc Brown
    Apr 6 at 6:21
19

A classic problem and the answer is: don't reference any static system functions like Date or File etc inside your class.

However. I would go further. Don't put calculated fields such as "Age" on your objects at all. Its just asking for trouble when you store or serialise the object.

so instead of:

class User {
    ...
    private static readonly MINIMUM_LEGAL_AGE: number = 21;
    private birthDate: Date

    ...

    hasMinimumLegalAge(currentDate: Date): boolean
    {
        //perform date difference between today - birthdate, check is user is above MINIMUM_LEGAL_AGE.
    }
}

Have:

class User {
    public birthDate: Date
}


class PurchaseAlcholInTexasRule 
{
    canPurchaseAlchol(User, TodaysDate, whateverOtherInfo) : boolean
    {
        ...whatever crazy laws they have in texas
    }
}
18

If your data always works under the assumption of the present time; then you don't need an external clock dependency. (That is not to say you can't implement one if you wish to e.g. be able to depend on a third party clock)

However, let's say that you have a hotel booking app where only adults are allowed to stay. Suppose you wish to allow 17-year-olds to make a booking provided that they will be 18 at the time of the booking.

Now, your domain logic is working not just in the present time, but in the future (will this person be an adult on this future date?), and now the external clock dependency makes sense to implement as the person class should not be keeping track of a booking or its dates; it should just tell you the person's age based on a date.

(The same principle applies when working in the past)

15

Looking at separation of concerns / single-responsibility principle for encapsulation, the following questions come to mind:

  • Why does the User object care if it is a legal age?
  • Who should be responsible for determining if the user object is a legal age?
  • The legal age for what?
  • Who is responsible for making sure the time used in determining age is accurate, including time zone corrections if that matters?

Using the real-world "driving" example: License (permission) to operate a motor vehicle is typically restricted by age, but the age in question varies by location. When a person applies for such a license, the relevant licensing authority typically does not simple ask the person "are you of legal age?" and take a "yes/no" answer. Instead, they typically want to make that determination themselves, based on the person's birthdate, some authoritative source for the current time, and the relevant laws (policy). The criteria for this determination is available to the person, so they can avoid wasting time going to the licensing authority in person and waiting for their application to be processed, but the person themself is not the one who grants that permission.

Like some other answers, this makes me think additional object types might help clear up responsibility in this interface.

Alexei Levenkov's answer before it was deleted, showed using a DateProvider (authoritative source of current time) and a Policy. If in your application, the answer to "Why does the User object care if it is a legal age?" is, as in our car example, "to avoid wasting resources on a request guaranteed to fail", then that policy class could include an accessor method providing its minimum age, or perhaps the earliest birthdate for which permission would be granted.

[EDIT]: That answer appears to have been deleted. The general idea for simple cases is something like this:

class DateProvider // or "interface DateProvider"
// if that is how your language of preference 
// describes purley abstract classes with only
// pure virtual methods
{
    public:
    virtual Date current_date() const = 0;
    // probably not necessary if your language declared as "interface"
    virtual ~DateProvider() = default;
};

class AgePolicy
{
   public:
   AgePolicy(TimeDuration legal_age)
   : minimum_age(legal_age)
   {}

   bool meets_age_requirement(User const & usr, DateProvider const & dp) 
   {
      // assuming your Date and TimeDuration classes have
      // proper operator overloads for this
      return dp.current_date() - usr.get_birthday() >= this->get_minimum_age();
   }

   // optional, in case User really needs to know this
   TimeDuration get_minimum_age() const {return minimum_age;}

   private:
   TimeDuration minimum_age;
};

To address supercat's comment about generalizing beyond age, you could instead set up the policy like:

class Policy
{
   public:
   virtual bool meets_requirement(User const &) const = 0;
   virtual ~Policy() = default;
};

class AgePolicy : public Policy
{
   // as previous AgePolicy, but now pass pointer or reference to 
   // DateProvider in constructor so it can saved as private data
   // to be used in the method call
};

You could then have an implementation of Policy that aggregates other concrete Policy implementations (location, insurance, etc) and whose method returns the logical "and" of each policy.

The advantage of having a separate policy object is that it provides a single answer to "The legal age for what?" Maybe today you care about "renting a car in Spain", but in five years your company has offices in the US and China, and now you care about those, too. Being able to inject those policy differences can make that more straightforward to update.

I know you were concerned in the comments about leaking encapsulation of the date, but I think that is inherent in this interface. If I ask the local drivers license authority to renew my driver's license, they'll want my current license number; that's just part of that interface. As a small bit of what is essentially "plain data" that has no functionality or behavior attached to it directly (in the case of the license number, it is just a glorified integer), I think that's fine; no worse to return an integer than a bool.

Dates are a little more complicated than plain integers in that there are a variety of possible formats and reference points, but if necessary, I think handling those differences is better solved with some sort of adapter object to convert between formats (in the simplest case, that would be applied when data enters or exits your program from users, files, system calls, third-party libraries, etc. and a single consistent representation is used within your code).

If you are concerned about the date being completely publicly accessible, you might be able to make use of the pass-key pattern to make access a little more restricted.

The advantage of having a separate DateProvider interface is that it provides a single answer to: "Who should be responsible for determining if the user object is a legal age?" If you currently only care about your local time, you can make a concrete implementation that queries some authoritative server for your local time. If you want to trust code running elsewhere to provide its own time (knowing that computer users can change their system clocks), you can make a class that does that. If you want a test class that returns a random date each time to see if the system is robust against absurd inputs, you can make that.

That example could be further extended to include some sort of permitting authority object, that makes use of a particular policy and date provider via aggregation or composition to enforce the state or action of whatever it is the user object is trying to get permission to do. Now we have a single answer to "Who should care if the user object is a legal age?" This provides a place to make sure the right combination of DateProvider and Policy are used; if you are enforcing rules in Tokyo and New York, not only the age limits, but the local date may differ, and you don't want to get those mixed up.

Depending on what the user is trying to do with that permission (and the capabilities of the language involved), maybe the authority object simply calls some function if permission would be granted, or maybe it provides the user with a passkey. In this case, the policy class could be changed to take a date as its input instead of a user, if you wanted (in which case the policy allows for jokes like "my car is old enough to drink now"). Something like:

class Authority
{ 
     public:
     // in languages that don't garbage-collect, be wary of
     // lifetime issues here
     Authority(DateProvider *, Policy *);
     // if you really want a bool 
     bool permits(User const &) const;
     // see the link for possible implementation details of the PassKey;
     // some other function or object that requires permission then
     // takes the PassKey as an argument
     PassKey request_permission(User const &) const;
     // where Callable could be a function pointer in languages
     // that support them, or an object with an overloaded
     // call operator in languages that have those, or
     // an interface with relevant method
     void call_if_permitted(User const &, Callable) const

     // private data members to store provider, policy
}; 

If we want a sanity check, we could also look at this from a data perspective. I know this is tagged object-oriented, but we're about to see how using other tools can suggest that we may be on the right track for that object-oriented design. Suppose we wanted to make these objects persistent by storing them in a database. Database normalization and object-relational mappings are topics unto themselves, but we'll keep it simple because we're only after a sanity check here.

  • It would probably make sense to store the birthdate in the user table, because each user is expected to have exactly one birthday, that can be looked up based on who the user is (whatever that primary key may be).
  • It probably does not make sense to store the current time in the table, because the duplicated data wastes space and provides an opportunity to fail to keep it updated properly, and has nothing to do with the primary key of the user table; that information probably belongs as the return value of some kind of subroutine or system call, maybe taking the relevant location to account for timezone differences if necessary.
  • It probably does not make sense to have some table-wide variable in the user table for legal age, because that data has nothing to do with whatever the primary key for users is.
  • It may, however, make sense to have some kind of policy table (say, minimum driving age by country) that stores the minimum driving age keyed to some code for the country.
  • The authority, if used, might by keyed on a combination of local time zone and country code, allowing lookups into the policy table, and using the time zone with the date call.

Object-relational mapping in general is not always an easy problem, and some problems just won't look similar, or correct, from both perspectives, but if you find an organization that does look right both ways, that's a good sign.

3
  • This is the answer. The user should be responsible for calculating the age (or providing the date of birth so that age can be calculated); determining whether that age is sufficient for some purpose is the responsibility of whoever enforces such rules. What if you need to check/enforce different ages for different purposes (e.g. drive at 16, vote at 18, buy alcohol at 21), or when enforcement rules change or get more complex (e.g. tobacco age increase from 18 to 21)? That's not really changing something about a user, so why should it require changes to the User class?
    – yoozer8
    Apr 6 at 2:24
  • 1
    A complication is that ability to buy age-restricted products might depend upon factors other than a user's birthdate. For example, whether because of law or policy, a business might sell tobacco products to people 18 years or older who would be eligible to purchase tobacco products in their state of residence, but refuse sales to 18-year-olds whose state of residence had a minimum age of 21.,
    – supercat
    Apr 6 at 22:25
  • 2
    @supercat Indeed, it may well make sense to continue passing the User object to the policy, so that additional information about the user can be queried if necessary. Apr 7 at 12:46
8

There is a third (very common) approach - use dependency injection to give User its dependency on DateService. Dependency injection is an extremely common way to make objects unit testable.

You don't need a fancy DI framework with automatic wiring to do DI, it can be (and often is) as simple as taking "pointer to IDateService" as an argument to the User constructor, where IDateService is an interface. In production you pass a real DateService (in whatever code initializes Users), in test setup you pass a mocked DateService.

For this example specifically, it might make sense to go with approach 2, because you often do care about the age of a user on a specific not-today date. But from a broader software design perspective this is the exception to the rule - usually a class's callers have no legitimate reason to know anything about most of its dependencies, and so you don't want to use approach 2 in general. That's the spirit of encapsulation.

6

TL;DR: Neither is the optimal approach. The User class should provide a function to get the user's current age (and optionally to get the user's age on a given date); some other object should be responsible for determining whether a given age is sufficient.


The MINIMUM_LEGAL_AGE isn't really information about a user (you've noticed this, which is why you've declared it static rather than an instance variable). Further, it's not actually information about users in general. As defined, it could be interpreted to mean that it is the minimum allowed age for users (i.e. all instances user of class User are guaranteed to have user.age >= User.MINIMUM_LEGAL_AGE).

It is a rule/requirement of some particular operation, and should be defined there. Let's consider this from the perspective of the Single-responsibility principle using a real-world example:

In the United States, the minimum legal age for purchase and consumption of alcohol is determined by state law. This means there are 50+ definitions of MINIMUM_LEGAL_AGE for this operation, depending on the context (the state, or non-state jurisdiction, where the operation is being attempted). The same is true in Canada. Should the User class be responsible for tracking the MINIMUM_LEGAL_AGE for all operations, and all variations on all operations? The class definition would quickly get unmanageably large.

In the 1980s, the U.S. passed a law that used federal funding to entice (or coerce, depending on your point of view) all of the states to raise the drinking age to 21. For some states, this was a change. For others, maybe it wasn't. Either way, it definitely did not change the birthdate (or any other vital information) about the people covered by the law. If you are making a code change, and the only change is the rule for performing a particular operation, does it make more sense for that code change to be in or adjacent to the implementation of the operation, or for the change to be in the User class?

In addition to the rules/requirements of this particular operation changing, what if you want to support other operations? There are age restrictions for getting a driver's license; these are also (in the U.S. at least) sate-level rather than federal. Should the User class have hundreds of MINIMUM_LEGAL_DRIVING_AGE_IN_QUEENSLAND and MINIMUM_LEGAL_PURCHASE_ALCOHOL_AGE_IN_FLORIDA constants, even though a significant portion of User instances may only ever be involved in operations requiring a small subset of such values?

I suggest Approach 3: define a class (or more likely an interface) for a RestrictedOperation that includes the function permittedForUser(user: User): boolean. If the operation requires a minimum age, it can query the User object for its age and check that it is sufficient (and you can easily configure several instances of similar operations with different age thresholds sharing the same implementation).

1

Clearly, the answer "are you of legal age now" will change over time, without any change to the object itself. So if I understand you correctly and you want to store the current date with the object, that's just awful.

In practice, you will not only need to know whether someone is of legal age right now, but also at other points of time. For example, a young person signs a contract. What's important is not whether they are an adult now, but whether they were an adult when they signed a contract. I book a hotel. It doesn't matter whether I'm of legal age now, but whether I'm of legal age when I want to use the hotel room with my girlfriend.

So you have a method that checks whether X is of legal age at a date that is given as a parameter, and then you may have an additional method that checks for the time "now" or possible give the method a default argument.

PS. The "CanDrive" method in the other answer is fatally flawed. 1. The UTC date doesn't matter, it's the local date. 2. Adding / subtracting years can fail which has in the past led to widely reported errors. 3. Birthdays are unusual. They are days not points in time, and they are independent of your time zone. As a result you may be legally 16 year old in one place but not in another.

0

The pragmatic solution to this problem is to make hasMinimumLegalAge(...) take the current date as an argument but have the argument default to using the current date if one is not provided by the caller.

0

There's a few separate issues in this question.

  1. Clarity of intent

Are you trying to provide a function that will decide whether the user is currently of legal age, or are you trying to write a function to decide this on any date?

If people will only ever call this method with Date=now(), there is no point even having the parameter. Just call your standard library's now() function inside the method. Current time is a rare case where global variables make sense, there is no need to encapsulate it. YAGNI is a best practice too.

If you want to support different dates, then your current approach is fine. However, in that case a better name is legalAgeAsOf(date). Going further, you might as well make a method called legalAgeDate() that returns the critical date, and the caller can then handle comparing it to now().

  1. Ease of testing

Contorting the API is not necessary for modern testing. If your hasMinimumLegalAge has to check now() internally, you can simply mock the standard library's now() during that test.

-1

This shouldn't be on the User object at all. What will you do when you release in Mexico, where the legal age is different? The User should know the birthdate. A Validator object of some sort should know the logic for whether a User is able to do something.

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