Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#170]: interfaces creation #467

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

Guryanov-Maksim
Copy link
Contributor

@Guryanov-Maksim Guryanov-Maksim commented Dec 24, 2023

#170

Создал интерфейсы для классов, чтобы реализовать ioc

Пара моментов, на которые стоит обратить внимание:

  1. В коде есть комментарии, начинающиеся с XXX. Там я задал уточняющие вопросы.
  2. Все методы классов, которые не являются публичными я сделал приватными

Copy link

github-actions bot commented Dec 24, 2023

Coverage report for apps/ligretto-gameplay-backend

St.
Category Percentage Covered / Total
🔴 Statements
48.35% (-1.45% 🔻)
352/728
🔴 Branches 25.24% 26/103
🔴 Functions 26.7% 55/206
🔴 Lines
45.73% (-1.75% 🔻)
289/632
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / game.service.ts
43.96% (-1.21% 🔻)
31.25% 27.59%
45.12% (-1.31% 🔻)
🔴
... / gameplay-controller.ts
22.73% (-3.36% 🔻)
0% 0%
19.05% (-3.68% 🔻)
🟢
... / games-controller.ts
81.82% (-0.53% 🔻)
81.25% 58.33%
81.25% (-0.57% 🔻)
🔴
... / gameplay.ts
17.02% (-4.98% 🔻)
0% 0%
13.33% (-5.42% 🔻)
🔴
... / handler.ts
34.38% (-7.29% 🔻)
0% 0%
31.03% (-8.36% 🔻)
🟢
... / game.repo.ts
86.21% (-0.46% 🔻)
100% 81.25%
85.71% (-0.65% 🔻)
🔴
... / playground.service.ts
21.62% (-2.06% 🔻)
0% 0%
17.65% (-2.35% 🔻)
🔴
... / playground.repo.ts
28.57% (-3.25% 🔻)
100% 0%
23.53% (-4.25% 🔻)
🔴
... / player.repo.ts
13.95% (-1.96% 🔻)
0% 0%
12.9% (-2.72% 🔻)
🔴
... / player.service.ts
14.29% (-1.71% 🔻)
0% 0%
11.9% (-2.05% 🔻)
🟡
... / user.repo.ts
66.67% (-1.52% 🔻)
0% 50%
64.71% (-1.96% 🔻)
🔴
... / user.service.ts
50% (-2.94% 🔻)
0% 40%
46.15% (-3.85% 🔻)
🔴
... / bot-controller.ts
56.25% (-2.57% 🔻)
100% 0%
50% (-3.33% 🔻)

Test suite run success

12 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from 3df9a2f

handleMessage: (socket: Socket, action: AnyAction) => void
}

// XXX: Is there the necessity of IController
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no
Lets try to remove @injectable()


connect(socketServer: Server) {
socketServer.use(authMiddleware).on('connection', socket => this.connectionHandler(socket))
}

public async connectionHandler(socket: Socket): Promise<void> {
// XXX: why is this method public? It is used only inside itself
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants