Adds two new time testing methods ('isFuture' and 'isPast') to the CakeTime Utility #1190

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@robertofrega
Contributor

Adds two new time testing methods ('isFuture' and 'isPast') to the CakeTime Utility.

Tests included.

@ceeram ceeram commented on the diff Mar 20, 2013
lib/Cake/Test/Case/Utility/CakeTimeTest.php
@@ -596,6 +596,56 @@ public function testIsToday() {
$result = $this->Time->isToday('-1 day');
$this->assertFalse($result);
}
+
+/**
+ * testIsFuture method
+ *
+ * @return void
+ */
+ public function testIsFuture() {
+ $result = $this->Time->isFuture('+1 month');
+ $this->assertTrue($result);
+ $result = $this->Time->isFuture('+1 days');
+ $this->assertTrue($result);
+ $result = $this->Time->isFuture('+1 minute');
+ $this->assertTrue($result);
+ $result = $this->Time->isFuture('+1 second');
ceeram
ceeram Mar 20, 2013 Member

indentation is off here and a few more occasions, use a single tab for indenting one level

@majna majna commented on the diff Mar 20, 2013
lib/Cake/Utility/CakeTime.php
@@ -465,6 +465,32 @@ public static function isToday($dateString, $timezone = null) {
$timestamp = self::fromString($dateString, $timezone);
return date('Y-m-d', $timestamp) == date('Y-m-d', time());
}
+
+/**
+ * Returns true if given datetime string is in the future.
+ *
+ * @param integer|string|DateTime $dateString UNIX timestamp, strtotime() valid string or DateTime object
+ * @param string|DateTimeZone $timezone Timezone string or DateTimeZone object
+ * @return boolean True if datetime string is in the future
+ * @link http://book.cakephp.org/2.0/en/core-libraries/helpers/time.html#testing-time
+ */
+ public static function isFuture($dateString, $timezone = null) {
+ $date = self::fromString($dateString, $timezone);
+ return date('Y-m-d H:i:s', $date) < date('Y-m-d H:i:s', time());
majna
majna Mar 20, 2013 Contributor

It's better to compare timestamps :)

return  $date < time();
robertofrega
robertofrega Mar 20, 2013 Contributor

It is the same way all of the other tests are made.

majna
majna Mar 21, 2013 Contributor

Others like isToday() assert if 2 strings are equal. When comparing strings numerically, PHP internally cast string to numeric, in this case integer number (year only). Also comparison operator is wrong in both methods, test case for isPast() and target branch.

markstory
markstory Mar 21, 2013 Owner

Comparing strings with < will compare the ascii values, I agree with majna that comparing timestamps is a better idea. This is not an issue when doing equality tests though.

Owner

Pull requests for new features should target the next release branch (2.4), but I can take care of that once the coding standards issues are fixed.

@markstory markstory added a commit that referenced this pull request Mar 31, 2013
@markstory markstory Merge branch 'caketime-improvements' into 2.4
Add CakeTime::isPast() and CakeTime::isFuture().

Closes #GH-1190
d681a19
Owner

Code fixed and merged into 2.4 in d681a19

@markstory markstory closed this Mar 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment