Nick/lesson19 #41
Nick/lesson19 #41
Conversation
Add field analizator
Calculate winner with saga
| </div> | ||
| ); | ||
|
|
||
| export const GameState = connect( |
There was a problem hiding this comment.
я хоть и не люблю хуки, но с функциональными компонентами они как-то лучше выглядят )
имхо
There was a problem hiding this comment.
А что тебя в этом файле смущает?) Что режет глаз?
There was a problem hiding this comment.
Пришли данные - компонент отрисовал, выглядит вроде проще некуда, на первый взгляд
There was a problem hiding this comment.
много возни с connect/mapStateToProps
ну то есть с хуками ее просто меньше и код плотнее
There was a problem hiding this comment.
Хорошо, попробую подумать
| @@ -0,0 +1,82 @@ | |||
| import { createSlice, PayloadAction } from "@reduxjs/toolkit"; | |||
There was a problem hiding this comment.
может файл slice.ts назваить? так то тут не только редьюсер
There was a problem hiding this comment.
Согласен, тут не только reducer, в соответствии с ducks https://github.com/erikras/ducks-modular-redux
Я привык называть данный файл reducer, можно назвать slice, мне не принципиально
Считаешь, стоит переименовать?
| const diagonal1: string[] = []; | ||
| const diagonal2: string[] = []; | ||
|
|
||
| for (let i = 0; i < gameField.length; i++) { |
There was a problem hiding this comment.
к слову - вроде нет ограничения на квадратность поля
плюс на больших полях считают не только главные диагонали (вроде бы)
There was a problem hiding this comment.
Это очень хороший поинт! Надо подумать )
There was a problem hiding this comment.
Добавлен код, который исправляет данную ситуацию
| moves: number; | ||
| winner?: string; | ||
| } = { | ||
| fieldSize: defaultFieldSize, |
There was a problem hiding this comment.
@saitonakamura как ты относиться к typeof initialState ? мне вот это дублирование нифига не нравится
There was a problem hiding this comment.
Поддерживаю) тоже хотел бы так, а так же нужно иметь возможность сделать поле не обязательным
winner?: string;
There was a problem hiding this comment.
winner: undefined as string | undefined ?
There was a problem hiding this comment.
@saitonakamura в этом случае он не будет требовать передать туда undefined по умолчанию? Попробую, спасибо
There was a problem hiding this comment.
Будет требовать, но это ж нормально, редьюсер же обычно спредит
Вообще мне что-то пока не очень заходит идея инференса из initialstate
Например нельзя выражать инварианты, много возни с юнионами как выше
Кажется что подход с отдельным стейтом более готовый к реальному коду, хоть и вербозно
src/components/Games/reducer.ts
Outdated
| ...state, | ||
| ...initialState, | ||
| }), | ||
| click: (state, { payload }: ClickActionType) => { |
There was a problem hiding this comment.
вот тут я сомневаюсь )
ClickActionType вроде фукнция же, ReturnType ClickActionType не нужно?
There was a problem hiding this comment.
PayloadAction вроде не функция, можешь проверить плиз?)
There was a problem hiding this comment.
@saitonakamura а что TS стайлгайд говорить про постфикс Type ? про префик I я помню
в примерах не вижу https://basarat.gitbook.io/typescript/styleguide#type
There was a problem hiding this comment.
у тс нет официального гайда как такового
но общепринятое примерно так
типы CamelCase
интерфейсы ICamelCase или CamelCase
дженерики TCamelCase
src/components/Games/fieldManager.ts
Outdated
| gameFieldState.map((vector) => | ||
| vector.reduce((acc, item) => { | ||
| if (item === mark) { | ||
| return ++acc; |
There was a problem hiding this comment.
Почему не acc + 1?
Никаких профитов от мутаций value type мы здесь не получаем вроде
There was a problem hiding this comment.
Да, спасибо, поправлю
|
|
||
| import { createEmptyGameField } from "./fieldManager"; | ||
|
|
||
| export enum GameStatus { |
There was a problem hiding this comment.
Предпочитаю string literal union, string based енамам: "New game" | "Play" | "Game over"
Енамы хороши для чисел, битовых флагов, но теряют эти преимущества в строках
При этом остаются проблемы
- их 4 вида (enum. const enum, declare enum, declare const enum), каждый немного отличается от другого
- некоторые виды не умеет траспилить бабель
- некоторые виды попадают в рантайм (хотя чаще всего они там не нужны)
- вот это не вызывает ошибки (с числами также)
enum GameStatus {
NewGame = "Play",
Play = "Play",
GameOver = "Play",
}- enum это не закрытое множество и доказать на уровне типов что ты обработал все возможные варианты нельзя
There was a problem hiding this comment.
Спасибо за такой подробный коммент!
| moves: number; | ||
| winner?: string; | ||
| } = { | ||
| fieldSize: defaultFieldSize, |
There was a problem hiding this comment.
winner: undefined as string | undefined ?
| <FieldWrapper> | ||
| {field?.map((row, y) => [ | ||
| ...row.map((filled: string, x) => ( | ||
| ...row?.map((filled: string, x) => ( |
There was a problem hiding this comment.
Не особо понял в каких случаях field или row может не быть
There was a problem hiding this comment.
Был сценарий, когда падал storybook, очень неприятно, потом долго пересобирает, лучше обработать
No description provided.