Можно как-то сделать рефакторинг данного кода?

Question

[Руслан Абсалямов]

То есть в переменную $time попадает строке в формате '01:30' типо такого. Дальше в конструкторе она делиться на

array (size=2)
  0 => 
    array (size=2)
      0 => int 0
      1 => int 1
  1 => 
    array (size=2)
      0 => int 3
      1 => int 0

class TimeModel
{
    private $hour1;
    private $hour2;
    private $minut1;
    private $minut2;

    /**
     * Переводиться в формате ММ:СС
     * TimeModel constructor.
     *
     * @param string $time
     */
    public function __construct(string $time)
    {
        if ($time[0] == '0'){
            $time = substr($time, 1);
        }
        $arDate = preg_split('/:/', $time);
        $this->hour1 = (int)$arDate[0][0];
        $this->hour2 = (int)$arDate[0][1] ?? null;
        $this->minut1 = (int)$arDate[1][0];
        $this->minut2 = (int)$arDate[1][1];
    }

    /**
     * Округление времени до пяти минут
     * @return string
     */
    public function getRounding() :string
    {
        $arTime = [];

        $arTime[0][0] = $this->hour1;
        $arTime[0][1] = $this->hour2;
        $arTime[1][0] = $this->minut1;
        $arTime[1][1] = $this->minut2;


        if ($this->minut2 >= 1 && $this->minut2 <= 4){
            $arTime[1][1] = 5;
        } else if ($this->minut2 >= 6 && $this->minut2 <= 9){
            if ($this->minut1 < 5) {
                $arTime[1][0] = $this->minut1 + 1;
            } else {
                if ($this->minut1 >= 5) {
                    if (!$this->hour2) {
                        if ($this->hour1 == 9){
                            $arTime[0][1] = 0;
                        }
                        $arTime[0][0] = $this->hour1 + 1;
                    } else {
                        if ($this->hour2 < 9){
                            $arTime[0][1] = $this->hour2 + 1;
                        } else {
                            $arTime[0][0] = 1;
                            $arTime[0][1] = 0;
                        }
                    }
                    $arTime[1][0] = 0;
                } else {
                    $arTime[0][1] = $this->minut1 + 1;
                    $arTime[1][0] = 0;
                }
            }
            $arTime[1][1] = 0;
        }

        return $arTime[0][0].$arTime[0][1].':'.$arTime[1][0].$arTime[1][1];
    }

}

метод getRounding мне кажется что он слишком большой, но пока что не пойму как можно его сократить.
Сам метод getRounding округляет цифру до 5 минут. Допустим, чтобы было не "14:01", а "14:00", не "7:23", а "7:25"
И возможно ли вообще его сократить?

Comments

[vyrkmod]

>То есть в переменную $time попадает строке '01:30' типо такого.
Что, простите? Это по-русски написано?

[Руслан Абсалямов]

vyrkmod, то есть это формат мм:сс

[Руслан Абсалямов]

vyrkmod, Обновил вопрос, чтобы было понятно

[vyrkmod]

Не очень понятно на кой вообще такой велосипед ваять. Если надо только округлять введённое время до пяти минут, это делается без плясок с четырьмя цифрами и упаковывается в одну функцию. Если класс собирается для множества операций типа "добавить n минут" или "вычислить разницу в минутах" - есть \DateTime.

[Руслан Абсалямов]

vyrkmod, То есть мне нужно добавлять 5 минут, если будет послежняя цифра 1-4 или 6-9, я правильно понял?

[vyrkmod]

if($min > 55) {
    $min = 55; // хотя можно и до следующего часа округлить
} else {
    $add = $min % 5 > 2 ? 5 : 0;
    $min = $min - $min % 5 + $add;
}

Answer 1

[Александр Новиков]

Если я правильно понял этот пахучий код - то это должно быть его полным эквивалентом
Округление в большую сторону до 5 минут/секунд

class TimeModel
{
    private $time;

    /**
     * Time in HH:MM format
     * TimeModel constructor.
     *
     * @param string $time
     */
    public function __construct(string $time)
    {
        $this->time = \DateTime::createFromFormat('H:i', $time);
    }

    /**
     * Округление времени до пяти минут
     * @return string
     */
    public function getRounding(): string
    {
        $nextFiveMinutesRound = ceil($this->time->getTimestamp() / 300) * 300;
        return (\DateTime::createFromFormat('U', $nextFive))->format('H:i');
    }

}

Comments

[Руслан Абсалямов]

Александр Новиков У меня на 3 часа раньше показывает, я попробовал как показано передать 3 аргументом тайм зону, но как я понял

Параметр timezone и текущая временная зона будут проигнорированы, если параметр time также содержит метку времени UNIX (то есть timestamp вида 946684800) или же указанную временную зону (то есть 2010-01-28T15:00:00+02:00).

скорее всего такой вариант не подойдет, потому что там уникс или передать туда сразу мм:сс формат?

[Александр Новиков]

Руслан Абсалямов, Код выше написан исходя из того, что в исходном

$arTime[0][0] = $this->hour1;
        $arTime[0][1] = $this->hour2;
        $arTime[1][0] = $this->minut1;
        $arTime[1][1] = $this->minut2;

судя по полям ожидаются часы и минуты.
Если часы не в 24 часовом формате, а в 12 - то можно заменить конструктор на

$this->time = \DateTime::createFromFormat('g:i', $time);

если речь о минутах и секундах - то

$this->time = \DateTime::createFromFormat('i:s', $time);

Если округление работает не только в большую сторону - то копать в сторону строки

$nextFiveMinutesRound = ceil($this->time->getTimestamp() / 300) * 300;

[Руслан Абсалямов]

Александр Новиков, Нет часы 24, просто есть там указание заказчика, типо если 07:00, то писать 7:00
А делить на 300 и умножить на 300, зачем нужны?

[Александр Новиков]

Руслан Абсалямов, посыл в любом случае такой - вместо того чтобы синтаксически анализировать строку с временем и пытаться по правилам её разобрать - надо использовать DateTime и внутренние механизмы работы с временем

[Дмитрий]

У меня на 3 часа раньше показывает, я попробовал как показано передать 3 аргументом тайм зону, но как я понял

Руслан Абсалямов, если я правильно понял, то эта ссылка Вам поможет.

[Александр Новиков]

Руслан Абсалямов, делени/умножение для окгругления к ближайшим 5 минутам (5 минут * 60 секунд * 2 диапазона, вверх и вниз), но этот кусок кода я взял уже готовый.

В общем, отрефакторить код из вопроса нельзя, переписать - можно и нужно

Порядок действий:

1) В конструкторе на основании принимаемого формата создавать объект DateTime и присваивать внутреннему полю
2) В getRounding реализовать логику, которую требует заказчик (по исходному коду и тексту вопроса непонятную). Использовать: php.net/manual/ru/class.datetime.php , может быть - php.net/manual/ru/datetime.diff.php
Также можно из DateTime переводить в timestamp и работать с округлениями уже там (int число на этом этапе куда удобнее и логичнее, чем кастомный двумерный массив)

Если округление нужно не в большую сторону, а в ближайшую - заменить на

$nextFive = round($this->time->getTimestamp() / 300) * 300;

Если указание для 07:00 писать 7:00 - то

return ltrim((\DateTime::createFromFormat('U', $nextFive))->format('H:i'), '0');

[Руслан Абсалямов]

Александр Новиков, То есть чтобы увеличить на 3 часа, мне лучше сделать так

return ltrim(\DateTime::createFromFormat('U', $nextFive)
            ->setTimezone(new DateTimeZone('Europe/Moscow'))
            ->format('H:i'), '0');

То есть весь код, если я правильно понял тебя, должен выглядеть примерно так

class TimeModel extends DateTime
{
    private $time;

    public function __construct(string $time)
    {
        $this->time = new DateTime($time);
    }

    public function getRound() :string
    {
        $nextFive = (ceil($this->time->getTimestamp() / 300) * 300);
        return ltrim(self::createFromFormat('U', $nextFive)
            ->setTimezone(new DateTimeZone('Europe/Moscow'))
            ->format('H:i'), '0');
    }
}

[Александр Новиков]

Руслан Абсалямов, в целом да, похоже на правду, но нужно проверить тестами.
Старый код тестами не покрыт? Если нет - надо покрыть, и только потом можно заменять на новый - тогда будет гарантия что новый код работает ровно так как надо и ничего не ломает
В любом случае, даже если будут расхождения - правки будут минимальными, основная идея такая

[Руслан Абсалямов]

Александр Новиков, Тестыми покрыть? это если имеется ввиду проверки делать? я пробовал менять на разное количество времени

[Александр Новиков]

Руслан Абсалямов, под тестами имеются в виду именно тесты
https://phpunit.de/getting-started/phpunit-7.html
Потому что мануальное тестирование программистом это вещь, которой доверять не стоит (подсознательно тестируется именно работающий функционал, в рамках именно запрограммированных сценариев), к тому же мануальное тестирование дороже обходится впоследствии