SIOnline icon indicating copy to clipboard operation
SIOnline copied to clipboard

Published 20 hours ago • verified publisher icon VladimirKhil

Рефакторинг кода

Open bitvalser opened this issue 3 years ago • 9 comments

Решился всё-таки оставиться пару своих замечаний по поводу веб клиента. На данный момент архитектра проекта не очень хорошо структурирована и плохо масштабируется (файлы на 800+ строк повергают в ужас). Вот пару предложений по улучшению:

  1. Хранилище. На данный момент редьюсеры, экешены и тд это просто один файл в проекте (reducer.ts, Actions.ts соответственно). Обычно хранилища разбивают на предметные области и каждый редьюсер отвечает за свою область данных, таким образом отслеживать изменения какой-то отдельно части гораздо проще. Причём где-то это разделение сделано, но в большинстве своём нет. Для этих целей и существует combineReducer. Пример использования: image image Для автомацизации создания экшенов и редьюсеров я бы предложил использовать typesafe-actions Также эта билиотека позволяет гораздо легче следить за типами в редьюсерах и экшенах, я заметил некоторую проблему с этим на данный момент.

  2. Экшены. Много места занимают mapStateToProps и mapDispatchToProps. В целом сократить такое кол-ва кода (либо вообще избавиться с помощью хуков useSelector и useDispatch) можно с помощью использования reselect библиотеки и использовании хуков. Но для этого нужно перейти на функциональные компоненты, что в принципе разумный шаг учитывая что сейчас функциональные компоненты проще в понимании и по коду получаются гораздо короче (а также более активно развиваются сейчас), опять же с помощью хуков и reselect библиотеки можно скоратить код и реиспользовать многие селекторы из reselect.

  3. Thunk. Я бы предложил также перейти на saga так как она более шире в использовании и в целом всю логику можно перенести туда и таким образом лучше разделять логику и представление компонента. Но это опцианально, thunk тоже со своими задачами справляется хоть и хуже на мой взгляд.

  4. Компоненты. Как я и писал рассмотреть преспективу перехода на функциональные компоненты. Также я бы предложил выделять под каждый компонент отдельную папку (так обычно и делается), сейчас всё находится в куче что не особо удобно. В целом также практиковать разбитие UI на более меньшие компоненты, это мало того что поможет в оптимизации, также код будет более читаемым.

  5. Типы. Выносить типы в отдельный types.ts файл чтобы не занимать место в файле компонента.

  6. Стили. Рассмотреть другой подход к стилизации, либо sass, либо в идеале styled-components - помимо множества фич в виде реиспользуемости кода и темизации, также имеет инкапсуляцию стилей, с чем есть проблема на данный момент.

  7. Структура папок. Пересмотреть структуру папок с учётом изменений выше, на данный момент это больше похоже как будто всё складывают в одну кучу.

  8. Очень много C# подобного кода, в JS есть свои практики и подходы. Например: Например for i циклы не используются, вместо них есть методы массива (forEach, map, filter и тд). Методы и переменные принято называть с малеьнкой буквы.

  9. Локализация. Лучше было бы спользовать i18n вместо собственного велосипеда.

Пока это того что нашёл беглым взглядом, список ещё будет обновляться.

Referres #2

bitvalser avatar Oct 29 '21 13:10 bitvalser

Идеи в целом здравые, только пока нет на это времени. А основной рефакторинг нужно проводить именно мне, чтобы разрабатывать дальше.

VladimirKhil avatar Oct 30 '21 06:10 VladimirKhil

Идеи в целом здравые, только пока нет на это времени. А основной рефакторинг нужно проводить именно мне, чтобы разрабатывать дальше.

А какая политика касательно форков? Я бы потихоньку как пета написал бы порт на svelte+TS, взяв отсюда только всю работу с АПИ. Не возбраняется?

CountTo25 avatar Nov 04 '21 20:11 CountTo25

Идеи в целом здравые, только пока нет на это времени. А основной рефакторинг нужно проводить именно мне, чтобы разрабатывать дальше.

А какая политика касательно форков? Я бы потихоньку как пета написал бы порт на svelte+TS, взяв отсюда только всю работу с АПИ. Не возбраняется?

@CountTo25 Автор выкладывал документацию по работе с апи, а также давал добро на использоание исходников в посте вк, там также найдёте ссылки на документацию. Правда зачем делать форк в вашем случае мне непонятно, учитывая что вы используете другой фреймворк, можно просто ориентироваться на исходники и писать с нуля код. Сам пишу мобильный клиент для игры в свободное время, так что всю инфу брал сугубо из доков. @VladimirKhil А вот что касается распорстранения приложения я не уверен насколько у нас развязаны руки? Понятно что будет указано автор оригинальной игры.

bitvalser avatar Nov 05 '21 16:11 bitvalser

Идеи в целом здравые, только пока нет на это времени. А основной рефакторинг нужно проводить именно мне, чтобы разрабатывать дальше.

А какая политика касательно форков? Я бы потихоньку как пета написал бы порт на svelte+TS, взяв отсюда только всю работу с АПИ. Не возбраняется?

@CountTo25 Автор выкладывал документацию по работе с апи, а также давал добро на использоание исходников в посте вк, там также найдёте ссылки на документацию. Правда зачем делать форк в вашем случае мне непонятно, учитывая что вы используете другой фреймворк, можно просто ориентироваться на исходники и писать с нуля код. Сам пишу мобильный клиент для игры в свободное время, так что всю инфу брал сугубо из доков. @VladimirKhil А вот что касается распорстранения приложения я не уверен насколько у нас развязаны руки? Понятно что будет указано автор оригинальной игры.

Предполагал, что чистого АПИ нет -- и придется из местных сорцов оторвать все подвязки к беку. Было бы проще форкнуть, убрать реакт и оставить голые модели с их гидрацией / функции для общения с беком.

Спасибо за информацию. Доки пока не вижу — если поделитесь прямой ссылкой, будет чудесно. Нет — нет, сам найду, кажется, просто с нахрапа не нашел.

CountTo25 avatar Nov 05 '21 17:11 CountTo25

Спасибо за информацию. Доки пока не вижу — если поделитесь прямой ссылкой, будет чудесно. Нет — нет, сам найду, кажется, просто с нахрапа не нашел.

@CountTo25 https://github.com/VladimirKhil/SI/wiki

bitvalser avatar Nov 05 '21 18:11 bitvalser

@CountTo25 @bitvalser можно свободно создавать форки и распространять их при указании автора исходной игры.

VladimirKhil avatar Nov 05 '21 19:11 VladimirKhil

  • [ ] move all Message constants to SIGameServer client folder
  • [ ] refactor messageProcessor into class
  • [ ] refactor state and reducers: https://github.com/VladimirKhil/SIOnline/issues/2
  • [ ] search for css alternative

VladimirKhil avatar Feb 04 '23 12:02 VladimirKhil

  1. Сделано.
  2. Будет сделано постепенно, по одному подклассу за раз.
  3. Потребности пока не видно. Выглядит некритично.
  4. Будет сделано, только не хотелось бы иметь универсальные имена файлов в папках (index.ts, style.css и пр.), так как это затрудняет навигацию между вкладками.
  5. В планах.
  6. В планах.
  7. Можно подумать.
  8. Не вижу ничего критичного в этом.
  9. Это отдельная библиотека. Мне кажется, тоже дело вкуса, как и со Thunk.

VladimirKhil avatar Sep 10 '23 09:09 VladimirKhil