Код ревью, как улучшить компонент React

Question

В проекте есть компонент Месяца, который генерирует подобную таблицу Вот кусок кода

const Month = () => {
const loader = useSelector(state => state.app.loader)
const currentMonth = useSelector(state => state.calendar.currentMonth)
const currentYear = useSelector(state => state.calendar.currentYear)
const tasks = useSelector(state => state.tasks.tasks)
const daysOfWeek = Object.keys(useSelector(state => state.calendar.daysOfWeek))

function createMonthGrid() {
    let date = new Date(currentYear, currentMonth, 1)
    let month = [], days = []

    for (let i = 0; i < getDay(date); i++) {
        days.push(<Day />);
    }

    while (date.getMonth() == currentMonth) {
        let tasksOfDay = tasks.filter(task => task.deadline === new Date(currentYear, currentMonth, date.getDate()).toLocaleDateString('en-CA'))

        if (date.getFullYear() === new Date().getFullYear() && date.getMonth() === new Date().getMonth() && date.getDate() === new Date().getDate()) {
            days.push(<Day key={date.getDate()} heading={date.getDate()} tasks={tasksOfDay} today={true} />)
        } else {
            days.push(<Day key={date.getDate()} heading={date.getDate()} tasks={tasksOfDay} />)
        }
        if (getDay(date) % 7 == 6) {
            month.push(<Week key={month.length} days={days} />)
            days = []
        }
        date.setDate(date.getDate() + 1);
    }

    if (getDay(date) != 0) {
        for (let i = getDay(date); i < 7; i++) {
            days.push(<Day />);
        }
    }

    month.push(<Week key={month.length} days={days} />)
    return month
}

function getDay(date) {
    let day = date.getDay();
    if (day === 0) day = 7;
    return day - 1;
}


if (loader) {
    return (
        <div className="loader">
            <div className="lds-ring"><div></div><div></div><div></div><div></div></div>
        </div>
    );
}

return (
    <div className="month">
        <Week days={daysOfWeek.map(day => <div className="week-day">{day}</div>)} />
        {createMonthGrid()}
    </div>
);}

Можно ли сделать код проще или улучшить его?

Answer 1

Возможно (не обязательно). Наполнение массива днями вынести в отдельную функцию. Т.е.
```
for (let i = 0; i < getDay(date); i++) {
    days.push(<Day />);
}
```
Вынести например в функцию fillArrayWithDays(). В итоге и метод будет сам за себя говорить и читать проще будет и кода меньше в методе
Таски на день тоже вынести в отдельную функцию. Т.е.
```
tasks.filter(task => task.deadline === new Date(currentYear, currentMonth, date.getDate()).toLocaleDateString('en-CA'))
```
можно вынести в getTasksOfTheDay или getDayTasks и в итоге чтобы было
```
let tasksOfDay = getDayTasks();
```
читать намного легче

Условие

if (date.getFullYear() === new Date().getFullYear() && date.getMonth() === new Date().getMonth() && date.getDate() === new Date().getDate()) {

ужасно. Никогда так делать не надо, т.к. читать это невероятно сложно. Выносите сложные конструкции в отдельные переменные, чтобы не путаться и работайте с ними. Пример:

if (IS_REGISTRATOR() && (($params.status === 'W' || $params.status === 'D' || $params.status === 'A') && $params.remark && (($params.subres_level == 0 && ($user_info->selected_title->tid == $params.boss || $user_info->selected_title->tid == $doc_signer_tid || !$params.usertid) || $params.subres_level > 0 && $user_info->selected_title->tid == $params.usertid))) { ... }

этот код будет читаться легче в таком виде:

$docIsInWorkAcceptOrDraft = ...;
$bossHasSignerPriviledge = ...;
$userCanSign = ...;

if ($docIsInWorkAcceptOrDraft && $bossHasSignerPriviledge && $userCanSign) {
  // ...
}

Есть строки
```
days.push(<Day key={date.getDate()} heading={date.getDate()} tasks={tasksOfDay} today={true} />)
```
и
```
days.push(<Day key={date.getDate()} heading={date.getDate()} tasks={tasksOfDay} />)
```
они отличаются только today={true}. Возможно (но не обязательно), при соблюдении пункта 3, если в итоге будет в итоге одна результирующая говорящая булева переменная, то это свойство можно будет неаписать через тернарник
getDay(date) % 7 == 6 и getDay(date) != 0 и if (day === 0) day = 7; --- никогда не пишите магических чисел! НИКОГДА. Создайте какую-нибудь константу или перечисление и оперируйте ими. Т.е. например
```
const WEEK_DAY_SUNDAY = 6;
const WEEK_DAY_MONDAY = 0;

getDay(date) % 7 == WEEK_DAY_SUNDAY
getDay(date) != WEEK_DAY_MONDAY
```
Не совсем понял что делает код
```
if (getDay(date) != 0) {
   for (let i = getDay(date); i < 7; i++) {
        days.push(<Day />);
    }
}
```
Но в целом условие можно вынести в отдельный метод (да да) или свойство с говорящим именем, а цикл, как и в п.1 вынести в другой метод. В итоге код может выглядеть примерно так:
```
if (currentDayIsSunday())
    fillArrayWithAllWeekDays();
```
Моё личное :))
- if (day === 0) day = 7; --- не пишите на одной строчке, пишите операцию на следующей. Как минимум мы всегда ожидаем, что тело под условием. Всегда. А тут внезапно иногда оно справа.... То есть невыполненные ожидания и читателю (другому разработчику) сложно будет поймать и прочитать код, когда он не соответствует одному выдержанному стилю
- return day - 1; --- слово return лучше всегда отделять от кода выше одной пустой строкой. За исключением случая, когда в методе только return. Например в случае
```
function getFullName() {
    return this.surname + ' ' + this.patronymic  + ' ' + this.name;
}
```
  перед return ставить лишнюю пробельную строку не имеет смысла. В остальных случаях - логически отделить стОит
if (loader) { --- используйте лучше булевы названия. Оно и читаться будет логичнее и как в книге. Например isLoading.... И тогда будет читаться так if (isLoading) {, т.е. буквально "Если загружается, то..." или "Если идёт загрузка то..."

БЛОГ НА HUSL

Код ревью, как улучшить компонент React

Ответы (1 шт):