2
\$\begingroup\$

I decided to write a few related examples of code, and honestly I don't know if I am overthinking, or just trying to find what is that right way to write functions and how to use them, so that codebase can be as clean as possible and function names are easy to understand and suitable for reuse.

Here's is what I have to demonstrate it, 4 separate functions:

<?php

// Function 0
function monthsPassed($startDate, $endDate) {
    $startDateObj = new DateTime($startDate);
    $endDateObj = new DateTime($endDate);
 
    $DateInterval = $endDateObj->diff($startDateObj); 
    $months = (($DateInterval->y) * 12) + ($DateInterval->m);
    
    return $months;
}

// Function 1
function monthsPassed_CurrentDate($startDate) {
    $startDateObj = new DateTime($startDate);
    $endDateObj = new DateTime('now');
 
    $DateInterval = $endDateObj->diff($startDateObj);
    $months = (($DateInterval->y) * 12) + ($DateInterval->m);
    
    return $months;
}

// Function 2
function hasMonthPassed($startDate, $endDate) {
    $monthsPassed = monthsPassed($startDate, $endDate);
    
    return $monthsPassed >= 1;
}

// Function 3
function hasMonthPassed_CurrentDate($startDate) {
    $monthsPassed = monthsPassed_CurrentDate($startDate);
    
    return $monthsPassed >= 1;
}

$startDate = '2000-01-11';
$endDate   = '2000-02-11';

// Function 0 Call
monthsPassed($startDate, $endDate);

// Function 1 Call
monthsPassed_CurrentDate($startDate);
    
// Function 2 Call
hasMonthPassed($startDate, $endDate);

// Function 3 Call
hasMonthPassed_CurrentDate($startDate);

Now these are fairly simple functions and I'm asking about code reuse and clarity which function should I use.

Let me explain my thinking.

These above all are the different variations of functions I came up with that calculate how many months passed between two dates, first two functions are more flexible, other next two are more suited for more specific case.

"Function 0" takes in two string arguments - $startDate, $endDate, returns $months (integer).

My observation: This function looks fairly flexible since I can pass any two valid dates.

"Function 1" takes one string argument - $startDate, returns $months (integer).

My observations: This function seems convenient when I need to compare it with current DateTime so I don't have to pass it manually; the function name is expressive.

"Function 2" takes two string arguments - $startDate, $endDate returns boolean.

My observations: This function is convenient to know whether at least one month passed between two dates. It also reuses 'monthsPassed' function, so it doesn't reimplement similar logic but promotes code reuse. The thing here is that I pass two dates.

"Function 3" takes one string argument - $startDate, returns boolean.

My observations: This is fairly the same as "Function 2" but I only pass $startDate, and that function takes care of comparing my $startDate with the current DateTime value, so I don't need to pass it and it promotes code reuse.

Now consider such scenario that I will have to use such checks in my codebase at least 10 times, where I have to check if at least one month has passed between $startDate and currentDate.

  • Which function would you use, which one would you prefer and why?
  • Are there any hard rules, or doesn't it matter?
  • Should I always aim to promote code reuse in my project and by having more general functions build more specific functions from that, or should I just straightforwardly implement exact specific logic in one function and do not rely on other functions to build my more specific function like hasMonthPassed?

It seems that these little functions could be handy, where you don't need to pass current date, but if flexibility is needed then more general function is suitable that can return $months number.

I think I'm struggling to find a good balance what should I do, having many small similar functions, and I hate to always do that guesswork if I should micro-optimize or over-engineer something small, meanwhile maybe I just need to use one way and stick to it, but would like to hear advice or opinions, what is the most viable option here.

As a side question, I also would like to ask in case of providing dates to functions (maybe it does apply to elsewhere to). Is it better practice to provide dates as strings, or should I provide DateTime objects outside of a function so functions do not depend on DateTime, which maybe is cleaner, but I am confused.

Should we be afraid of relying on built-in PHP classes, objects, methods, functions and using them inside of functions, or should we always strive to pass objects, other dependencies no matter what they are only outside of functions?

To me it looks cleaner to pass new DateTime() to arguments, meanwhile I am not sure, because it is a built-in PHP class, so I don't feel like it would give me so much benefit of passing DateTime outside of a function, since even if I can pass other implementations that could deal with DateTime it does not guarantee that they will come with the same properties or methods, so it's like all this clean stuff is for no good use if you would have to use different methods inside of your function if you use some kind of other DateTime library.

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

You have a bucket load of questions in your question. 🙂 I'm not sure I'll be able to answer them all, but I will give it a try. First the code. Things I notice immediately are:

  • Function 1 repeats function 0 with only a slight difference.
  • Function 3 doesn't use function 2, which would have made sense.
  • Function names combine camelCase and snake_case.
  • Use of parenthesis where they are not needed.
  • The use of the variable $months inside the functions doesn't make much sense.
  • Not using default values for parameters, see code below.

Incorporating these changes gives:

<?php

// Function 0 & 1
function monthsBetween($date1, $date2 = NULL) {
    if (is_null($date2)) {
        $date2 = new DateTime("now");
    }
    $interval = $date2->diff($date1); 
    return 12 * $interval->y + $interval->m;
}

// Function 2 & 3
function hasMonthBetween($date1, $date2 = NULL) {
    return monthsBetween($date1, $date2) > 0;
}

$startDate = new DateTime('2000-01-11');
$endDate   = new DateTime('2000-02-11');

// Function 0 Call
echo monthsBetween($startDate, $endDate) . PHP_EOL;

// Function 1 Call
echo monthsBetween($startDate) . PHP_EOL;

// Function 2 Call
echo hasMonthBetween($startDate, $endDate) . PHP_EOL;

// Function 3 Call
echo hasMonthBetween($startDate) . PHP_EOL;

DEMO: https://3v4l.org/WioCJ

Obviously I have chosen to turn stringy dates into PHP objects before using these functions. The reason is obvious, when you look at the code. Another good argument for this is that you often need to make sure stringy dates are valid before you use them, and converting them to objects is one way of doing this.

Notice how I can merge 2 function together because of the use of NULL as the default value for the second argument.

I renamed the functions slightly. I agree that "passed" and "between" are quite similar, but I felt that "between" more clearly describes that these functions look at the interval "between" the dates.

Now, given this, let me look at your questions again:

I think that hasMonthBetween($date1, $date2) and monthsBetween($date1, $date2) > 0 look very similar. As Your Common Sense already said; It's up to you. I'm always in favor of general functions and code reuse. This doesn't mean you cannot have a function like hasMonthBetween(). It depends on what your code needs to do. The function makes perfect sense when the question, whether two dates have a month between them, is important in your software.

I prefer shorter functions, because they are easier to read and understand. It's really time to refactor when a function is longer than what fits on your computer screen. Shorter functions are often also easier to test. If this means you'll have more functions then so be it.

Built-in PHP classes, objects, methods, and functions are part of the PHP language. With some exceptions, which are always documented in the manual, you can use them freely everywhere. Don't view them as risky "dependencies". Of course, some features of PHP do get deprecated over time, but it's hard to predict which those will be, and you shouldn't be held hostage by this.

Finally:

You can use Type declarations in your function declarations:

function monthsBetween(DateTime $date1, DateTime $date2 = NULL)

DEMO: https://3v4l.org/68Qur

This way you'll get an error message when you supply a stringy date or another type of object.

Your functions don't need them, but have a look at DateTimeImmutable, they are preferred because the normal DateTime can often cause unexpected effects when you use them as function arguments.

\$\endgroup\$
2
\$\begingroup\$

First of all, it's very right questions to ask. Every developer should ask such questions.

Regarding particular code, I find these function a bit redundant, though it's entirely a matter of taste. Given your goal is a readable code, a function name is not the only way to achieve it. To me, an expression

monthsPassed() > 0

is no less readable than explicit hasMonthsPassed()

The same goes for the current date. If you define a constant, CURRENT_DATE (or a function, for that matter, getCurrentDate()), you can pass it as a second parameter

monthsPassed($startDate, getCurrentDate()) > 0

to me is no less readable than

hasMonthPassedCurrentDate($startDate) 

but in the end it's entirely up to you, which variant to use.

Regarding having many small functions, it's not a problem. Besides, you can wrap them up in a class.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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