01 leap year homework#4
01 leap year homework#4tolikheha wants to merge 13 commits intodriverdevteam:masterfrom tolikheha:01_leap_year
Conversation
| EXPECT_EQ(false, is_leap_year(1997)); | ||
| } | ||
|
|
||
| TEST(leap_year, test_1996) |
There was a problem hiding this comment.
название теста ни о чем не говорит. Из этого имени нельзя понять, что действительно разработчик хочет проверить
| bool is_leap_year(uint32_t year) | ||
| { | ||
| if (year % 4 == 0) | ||
| if (year % 4 == 0 && year % 100 != 0) |
There was a problem hiding this comment.
какой-то тест поломался? Если нет никакого обснования добавлять (красный тест), то это не стоит делать
|
|
||
| bool is_leap_year(uint32_t year) | ||
| { | ||
| if (year % 400 == 0) |
There was a problem hiding this comment.
по какой причине не понятно хачем это добавилось - были красные тесты? получается изменяем много, а тестов нет
There was a problem hiding this comment.
А тут після фікса теста для 100 поламався тест для 2000
| return true; | ||
|
|
||
| if (year % 4 == 0 && year % 100 != 0) | ||
| if (year % 4 == 0) |
There was a problem hiding this comment.
какие были подставы для рефакторинга? тестов я не видел
There was a problem hiding this comment.
При додані нового теста докостилювалися нові умови в if. Це хіба не підстава для рефакторинга?
There was a problem hiding this comment.
да, но в коммите я не видел тестов, предыдущее изменение тоже рефакторинг
There was a problem hiding this comment.
А до якого коміта прив'язані ці коменти? Бо в мене попередній коміт - це фікс теста для 4. І після теста я зарефакторив умову, тому що всі попередні числа ділились на 4.
| @@ -16,12 +16,15 @@ If your language provides a method in the standard library that does this look-u | |||
|
|
|||
| bool is_leap_year(uint32_t year) | |||
There was a problem hiding this comment.
финальный рефакторинг не закончен, окончательный вариант функции +- вот таким может быть
return (year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0);
There was a problem hiding this comment.
Але ж це не найпростіше рішення на даному етапі
There was a problem hiding this comment.
это был бы последний этап - самый последний рефакторинг
No description provided.