Why is this addition function sometimes not working in PHP?

Tags: , , , ,



I’m currently writing a class to manage time in a specific format. The output is always in this format: 0000:00:0:00:00 which stands for YYYY:WW:D:HH:MM. I have a function to add time and it seemed to be working perfectly until I tried to call it several times.

I used this code to add the time, and it should have returned 0000:00:0:00:42 but it returned 0000:00:0:03:60.

Here’s my full class. Code Demo on https://3v4l.org/YiEQF

<?php

namespace Corecomponenttime;

final class TimeFactory {

    /** @var int */
    private $year;
    /** @var int */
    private $weekNumber;
    /** @var int */
    private $day;
    /** @var int */
    private $hour;
    /** @var int */
    private $minute;

    /**
     * @param string $formatted
     * @return static
     */
    public static function createFromString(string $formatted): self {
        if (!preg_match('/^(d{4}):(d{2}):(d{1}):(d{2}):(d{2})$/', $formatted, $matches)) {
            throw new InvalidArgumentException('Invalid currency format.');
        }
        return new self(...array_slice($matches, 1));
    }

   
    /**
     * TimeFactory constructor.
     * @param int $year
     * @param int $weekNumber
     * @param int $day
     * @param int $hour
     * @param int $minute
     */
    public function __construct(int $year, int $weekNumber, int $day, int $hour, int $minute)
    {
        $this->year = $year;
        $this->weekNumber = $weekNumber;
        $this->day = $day;
        $this->hour = $hour;
        $this->minute = $minute;
    }

    /**
     * @param int $addedYears
     * @return bool
     */
    public function addYears(int $addedYears): bool {
        $newYears = $this->year + $addedYears;
        if($newYears > 9999) {
            return false;
        } else {
            $this->year += $newYears;
        }
        return true;
    }

    /**
     * @param int $addedWeeks
     */
    public function addWeeks(int $addedWeeks): void {
        $newWeeks = $this->weekNumber + $addedWeeks;
        if($newWeeks >= 52) {
            $years = round($newWeeks / 52, 0, PHP_ROUND_HALF_DOWN);
            $this->addYears($years);
        }
        $this->weekNumber += $newWeeks % 52;
    }

    /**
     * @param int $addedDays
     */
    public function addDays(int $addedDays): void {
        $newDays = $this->day + $addedDays;
        if($newDays >= 7) {
            $weeks = round($newDays / 7, 0, PHP_ROUND_HALF_DOWN);
            $this->addWeeks($weeks);
        }
        $this->day += $newDays % 7;
    }

    /**
     * @param int $addedHours
     */
    public function addHours(int $addedHours): void {
        $newHours = $this->hour + $addedHours;
        if($newHours >= 24) {
            $days = round($newHours / 24, 0, PHP_ROUND_HALF_DOWN);
            $this->addDays($days);
        }
        $this->hour += $newHours % 24;
    }

    /**
     * @param int $addedMinutes
     */
    public function addMinutes(int $addedMinutes): void {
        $newMinutes = $this->minute + $addedMinutes;
        if($newMinutes >= 60) {
            $hours = round($newMinutes / 60, 0, PHP_ROUND_HALF_DOWN);
            $this->addHours($hours);
        }
        $this->minute += $newMinutes % 60;
    }

    /**
     * @return int
     */
    public function getYears(): int {
        return $this->year;
    }

    /**
     * @return int
     */
    public function getWeeks(): int {
        return $this->weekNumber;
    }

    /**
     * @return int
     */
    public function getDays(): int {
        return $this->day;
    }

    /**
     * @return int
     */
    public function getHours(): int {
        return $this->hour;
    }

    /**
     * @return int
     */
    public function getMinutes(): int {
        return $this->minute;
    }

    public function reduceTime(Int $amount, String $type) : bool {
        switch($type) {
            case "m":
                if($this->getMinutes() >= $amount) {
                    $this->minute -= $amount;
                    return true;
                } else {
                    if($this->getHours() > 0) {
                        $this->hour -= 1;
                        $minutesToAdd = 60 - $amount;
                        $this->addMinutes($minutesToAdd);
                        return true;
                    } elseif ($this->getDays() > 0) {
                        $this->day -= 1;
                        $hoursToAdd = 23;
                        $minutesToAdd = 60 - $amount;
                        $this->addHours($hoursToAdd);
                        $this->addMinutes($minutesToAdd);
                        return true;
                    } elseif ($this->getWeeks() > 0) {
                        $this->weekNumber -= 1;
                        $daysToAdd = 6;
                        $hoursToAdd = 23;
                        $minutesToAdd = 60 - $amount;
                        $this->addDays($daysToAdd);
                        $this->addHours($hoursToAdd);
                        $this->addMinutes($minutesToAdd);
                        return true;
                    }  elseif ($this->getYears() > 0) {
                        $this->year -= 1;
                        $weeksToAdd = 51;
                        $daysToAdd = 6;
                        $hoursToAdd = 23;
                        $minutesToAdd = 60 - $amount;
                        $this->addWeeks($weeksToAdd);
                        $this->addDays($daysToAdd);
                        $this->addHours($hoursToAdd);
                        $this->addMinutes($minutesToAdd);
                        return true;
                    }
                }
                break;
            case "h":
                if($this->getHours() >= $amount) {
                    $this->hour -= $amount;
                    return true;
                } else {
                    if ($this->getDays() > 0) {
                        $this->day -= 1;
                        $hoursToAdd = 24 - $amount;
                        $this->addHours($hoursToAdd);
                        return true;
                    } elseif ($this->getWeeks() > 0) {
                        $this->weekNumber -= 1;
                        $daysToAdd = 6;
                        $hoursToAdd = 24 - $amount;
                        $this->addDays($daysToAdd);
                        $this->addHours($hoursToAdd);
                        return true;
                    }  elseif ($this->getYears() > 0) {
                        $this->year -= 1;
                        $weeksToAdd = 51;
                        $daysToAdd = 6;
                        $hoursToAdd = 24 - $amount;
                        $this->addWeeks($weeksToAdd);
                        $this->addDays($daysToAdd);
                        $this->addHours($hoursToAdd);
                        return true;
                    }
                }
                break;
            case "d":
                if($this->getDays() >= $amount) {
                    $this->day -= $amount;
                    return true;
                } else {
                    if ($this->getWeeks() > 0) {
                        $this->weekNumber--;
                        $daysToAdd = 7 - $amount;
                        $this->addDays($daysToAdd);
                        return true;
                    } elseif ($this->getYears() > 0) {
                        $this->year--;
                        $weeksToAdd = 51;
                        $daysToAdd = 7 - $amount;
                        $this->addWeeks($weeksToAdd);
                        $this->addDays($daysToAdd);
                        return true;
                    }
                }
                break;
            case "w":
                if($this->getWeeks() >= $amount) {
                    $this->weekNumber -= $amount;
                    return true;
                } else {
                    if ($this->getYears() > 0) {
                        $weeksToAdd = 52 - $amount;
                        $this->addWeeks($weeksToAdd);
                        return true;
                    }
                }
                break;
            case "y":
                if($this->getYears() >= $amount) {
                    $this->year -= $amount;
                    return true;
                }
        }
        return false;
    }
    /**
     * @return string
     */
    public function __toString(): string {
        return sprintf('%04d:%02d:%d:%02d:%02d', $this->year, $this->weekNumber, $this->day, $this->hour, $this->minute);
    }
}
$factory = TimeFactory::createFromString("0000:00:0:00:00");
$factory->addMinutes(2);
$factory->addMinutes(4);
$factory->addMinutes(6);
$factory->addMinutes(8);
$factory->addMinutes(10);
$factory->addMinutes(12);

echo $factory;
echo "n" . $factory->getMinutes();
echo "n" . $factory->getHours();
echo "n" . $factory->getDays();
echo "n" . $factory->getWeeks();
echo "n" . $factory->getYears();

Any help would be much appreciated!

Answer

Not sure if the overall process is the best method of achieving what you want, but this is the problem ( I think ).

Just using the minute code as an example…

public function addMinutes(int $addedMinutes): void {
    $newMinutes = $this->minute + $addedMinutes;
    if($newMinutes >= 60) {
        $hours = round($newMinutes / 60, 0, PHP_ROUND_HALF_DOWN);
        $this->addHours($hours);
    }
    $this->minute += $newMinutes % 60;
}

So first you add the new minutes to the existing time…

$newMinutes = $this->minute + $addedMinutes;

process the hour rollover and then you add the new minutes field again…

$this->minute += $newMinutes % 60;

This is repeated at each layer.

So I suggest something like

public function addMinutes(int $addedMinutes): void {
    $newMinutes = $this->minute + $addedMinutes;
    if($newMinutes >= 60) {
        $hours = floor($newMinutes / 60);
        $this->addHours($hours);
        $newMinutes -= ($hours * 60);
    }
    $this->minute = $newMinutes;
}

This does similar processing, but the last part is just assigning the new minutes, rather than adding them again.

Note that I’ve also changed the way that the next level is calculated – using floor() instead of round().



Source: stackoverflow