-
Notifications
You must be signed in to change notification settings - Fork 10
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
[김희선]w3_리뷰요청_server의 model,controller구현 #66
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/* eslint-disable no-underscore-dangle */ | ||
import socketio from 'socket.io-client'; | ||
import EVENT from '../constants/socket-event'; | ||
|
||
class SocketContainer { | ||
constructor() { | ||
this.socket = undefined; | ||
this.connect(); //this.socket = socketio.connect('http://localhost:3000');이 들어가는 것보다 이렇게 함수를 호출하는 것이 좋을까요? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 생각했을때에는 함수로 사용하는 이유는 재사용이 가장 큰 이유라 생각해요! 만약 한번만 사용하게 되면 굳이 함수로 빼지 않아도 된다고 생각해요! |
||
} | ||
|
||
connect() { | ||
this.socket = socketio.connect('http://localhost:3000'); | ||
} | ||
|
||
disconnect() { | ||
this.socket.disconnect(); | ||
} | ||
|
||
onEnterRoom(callback) { // on을 해주는 부분을 socket파일에 넣고 싶어서 callback을 받아서 실행하는데 이 구조는 괜찮나요? | ||
this.socket.on(EVENT.ENTER_ROOM, (data) => callback(data)); | ||
} | ||
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이렇게 Wrapping해서 많이 사용해요! 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이렇게 사용하려면 callback 파라메터가 함수인지 확인하는 방어 코드가 있으면 좋겠네요! underscore, lodash를 사용하면 쉽게 사용할수 있을꺼에요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 그건 찾아봐서 적용하겠습니다. 감사합니다! |
||
|
||
onEnterPlayer(callback) { | ||
this.socket.on(EVENT.ENTER_PLAYER, (data) => callback(data)); | ||
} | ||
|
||
emitStartGame() { | ||
this.socket.emit('start_game'); // 보통 socket event name은 '_'를 많이 쓰는지 궁금합니다! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 일반적으로 white space를 사용한다는데... 이 방법이 맞는지 모르겠네요! 나중에 알게되면 어떻게 사용하는지 멘션 달아주면 좋겠어요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 알겠습니다! |
||
} | ||
|
||
onQuizList(callback) { | ||
this.socket.on('get_quiz_list', (data) => callback(data)); | ||
} | ||
} | ||
|
||
const socket = new SocketContainer(); | ||
|
||
export default socket; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import createError from 'http-errors'; | ||
import express from 'express'; | ||
import logger from 'morgan'; | ||
import socketIo from 'socket.io'; | ||
import {} from 'dotenv/config'; | ||
import indexRouter from './routes/index'; | ||
import gameController from './controller'; | ||
|
||
const app = express(); | ||
|
||
app.io = socketIo(); | ||
|
||
app.use(logger('dev')); | ||
app.use(express.json()); | ||
app.use(express.urlencoded({ extended: false })); | ||
|
||
app.use('/', indexRouter); | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 불필요한 공백은 제거해주세요~ |
||
app.use((req, res, next) => { | ||
next(createError(404)); | ||
}); | ||
|
||
app.use((err, req, res) => { | ||
if (req.app.get('env') === 'development') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앗 이건 express-generator에서 만들어준 것입니다..!ㅎㅎ |
||
res.status(err.status || 500); | ||
res.send({ message: err.message }); | ||
} | ||
}); | ||
|
||
app.io.on('connection', async (socket) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. app.io에 소켓을 할당 했다면 property 이름은 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넵 알겠습니다! 좋은 것 같아요! |
||
console.log('a user connected'); | ||
await gameController.enterPlayer(socket); | ||
}); | ||
|
||
export default app; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import quizFinder from '../database/quiz'; | ||
import Room from '../models/room'; | ||
|
||
class GameController { | ||
constructor() { | ||
this.players = []; | ||
this.rooms = []; | ||
this.rooms.push(new Room()); | ||
} | ||
|
||
async enterPlayer(socket) { | ||
this.players.push(socket); | ||
this.bindPlayerEvents(socket); | ||
|
||
const otherCharacters = this.rooms[0].getExistCharacters(); //방은 하나라고 가정하고 rooms[0]번째 방을 이용합니다. | ||
const character = await this.rooms[0].enterNewPlayer(); | ||
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이건 test로 쓴거고 rooms라는 배열에 많은 방들이 할당될 거라서 이건 로직에 맞게 이번주에 수정하겠습니다! |
||
socket.emit('enter_room', { character, otherCharacters }); | ||
|
||
this.players.forEach(async (player) => { | ||
if (player === socket) return; | ||
player.emit('enter_new_player', character); | ||
}); | ||
} | ||
|
||
async startRoomRound(roomIdx) { | ||
if (this.rooms[roomIdx].hasNoMoreQuiz()) { | ||
const quizList = await quizFinder.getQuizList(); | ||
this.rooms[roomIdx].addQuizList(quizList); | ||
} | ||
this.players.forEach(async (player) => { | ||
const roundValue = await this.rooms[roomIdx].startNewRound(); | ||
player.emit('start_round', roundValue); | ||
}); | ||
} | ||
|
||
bindPlayerEvents(socket) { | ||
socket.on('start_game', () => { | ||
// this.startRoomRound(0); // 문제 하나만 넘겨주는 logic | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용하지 않는 주석은 제거해 주세요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넵! |
||
|
||
this.players.forEach(async (player) => { // 문제 10개를 배열로 넘겨주는 logic | ||
player.emit('get_quiz_list', await quizFinder.getQuizList()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 보통 get과 같은 이름은 객체에서 프로퍼티를 가져오는 등 가벼운 작업에서 많이 사용되는 네이밍 룰이에요! 데이터베이스에서 가져오는 거라면 fetch라는 네이밍을 사용하는게 더욱 명확한 이름이라 생각돼요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 그렇군요. 네 알겠습니다 감사합니다! |
||
}); | ||
}); | ||
} | ||
} | ||
|
||
const gameController = new GameController(); | ||
|
||
export default gameController; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import getDataFromDB from './util'; | ||
/* eslint-disable */ | ||
export default { | ||
getQuizList: async() => await getDataFromDB(`SELECT * FROM Quiz ORDER BY RAND() LIMIT 10;`) | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import pool from './connection'; | ||
|
||
const getDataFromDB = async (sql) => { //이런식으로 db에서 pool.query하는 부분을 util로 빼서 사용하려고 합니다. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 방식이라 생각해요! 👍 wrapping 함수를 만들어 더욱 편하게 사용하는 것은 지향하는 방법이에요! 만약 query를 하고 싶다면 네이밍은 query로 하는게 좋을것 같아요! 그리고 ORM을 참고해서 데이터베이스 유틸을 만들어보는것도 추천해요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네. 네이밍은 수정하고 ORM참고해서 자주 쓰일만한 것들은 util에 추가해볼게요! |
||
const [rows] = await pool.query(sql); | ||
return rows; | ||
}; | ||
|
||
export default getDataFromDB; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import imageFinder from '../database/image'; | ||
|
||
class Player { | ||
constructor() { | ||
this.character = undefined; | ||
} | ||
|
||
async makeCharacter(indexX, indexY) { | ||
[this.character] = await imageFinder.getRandomCharacter(); | ||
[this.character.indexX, this.character.indexY] = [indexX, indexY]; | ||
return this.character; | ||
} | ||
} | ||
|
||
export default Player; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import Player from './player'; | ||
|
||
class Room { | ||
constructor() { | ||
this.quizs = []; | ||
this.currentRound = 0; | ||
this.currentQuiz = {}; | ||
this.fieldRow = 8; | ||
this.fieldColumn = 16; | ||
this.playerList = []; | ||
this.indexOfPlayers = Array(this.fieldColumn).fill(null).map(() => Array(this.fieldRow)); | ||
} | ||
|
||
/** | ||
* @returns {Object{quiz, charactersLocation:Array.<indexX, indexY>, countTime}} | ||
*/ | ||
async startNewRound() { | ||
this.setToCurrentQuiz(); | ||
this.currentRound += 1; | ||
|
||
return { quiz: this.currentQuiz, charactersLocation: [], countTime: 60 }; | ||
} | ||
|
||
addQuizList(quizs) { | ||
this.quizs = [...this.quizs, ...quizs]; | ||
} | ||
|
||
setToCurrentQuiz() { | ||
this.currentQuiz = this.quizs[this.currentRound]; | ||
} | ||
|
||
hasNoMoreQuiz() { | ||
return this.quizs[this.currentRound] === undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lodash, underscore등을 사용하면 명시적으로 비교할수 있을꺼에요! 예를들어 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 감사합니다. 코드리뷰로 알게 된 부분이 많은 것 같아요. 말씀하신대로 사용할만한 라이브러리도 살펴보고 더 좋은 방향내주신 부분은 수정해보려고 합니다. 늦은 시간에 코드리뷰해주셔서 감사합니다! |
||
} | ||
|
||
async enterNewPlayer() { | ||
const player = new Player(); | ||
const character = await this.assignCharacterToPlayer(player); | ||
return character; | ||
} | ||
|
||
async assignCharacterToPlayer(player) { | ||
const [indexX, indexY] = this.getRandomIndex(); | ||
const character = await player.makeCharacter(indexX, indexY); | ||
this.indexOfPlayers[indexX][indexY] = player; | ||
this.playerList.push(player); | ||
return character; | ||
} | ||
|
||
getExistCharacters() { | ||
return this.playerList.map((p) => p.character); | ||
} | ||
|
||
getRandomIndex() { | ||
let indexX; | ||
let indexY; | ||
do { | ||
indexX = Math.floor(Math.random() * (this.fieldColumn)); | ||
indexY = Math.floor(Math.random() * (this.fieldRow)); | ||
} while (this.indexOfPlayers[indexX][indexY]); | ||
return [indexX, indexY]; | ||
} | ||
} | ||
|
||
export default Room; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.socket; 이렇게 할당하면 자동으로
undefined
이 할당 될꺼에요!~