#30 웹소켓을 이용한 1:1 채팅 구현 #32

junshock5 · 2020-08-24T10:58:50Z

No description provided.

- WebSocket 의존성 추가 - WebSocketConfig 추가 - json 의존성 추가 - SocketHandler class 추가 - Chatting Controller 추가

- 채팅 jsp 처리를 위한 servlet추가 - ModelAndView 사용 및 chat.jsp 추가 - spring.mvc.view 설정

- json-simple 의존성 추가 - sessionID값을 기준으로 채팅 상대방과 분리되게 JSP 변경 - 채팅 message jsonToObjectParser 함수로 파싱 되게 변경

- WebSocketHandlerRegistry에 방번호까지 등록 - 채팅방 생성, 이동, 정보 가져오는 method controller에 생성 - 메세지 전송 method에서 세션정보를 갖는 List를 이용해 해당 방에만 유효한 세션을 골라 발송하게 변경 - 방 생성 또는 참여 웹소켓 연결시 방의 인원을 조사후 방번호와 세션을 추가한다. - 방 퇴장 웹소켓 종료시 해당 세션을 삭제한다. - 채팅방, 방입장 jsp 작성

- messageType이 BinaryMessage인 데이터 디렉터리에 저장 및 발송 - chat.jsp에 웹소켓을 통해 fileSend함수 생성

f-lab-dev · 2020-08-25T12:31:27Z

이 부분은 멘토링시에 말씀드린 내용대로 전체 재검토 부탁드려요~

- RequestMapping 으로 되어있던 api를 http method에 맞게 변경 - api에 맞는 uri 패턴으로 변경 - uri 패턴 변경에 따른 room.jsp 변경 - SoketHandler.java 파일에 필요없는 변수 삭제 및 Thread-Safe하기 위해 전역변수를 지역변수로 변경

- WebSocketConfig 클래스 관련 주석 추가 - ChattingController에 REST URI 규약 적용 - roomDTO builder 패턴을 이용한 생성되게 변경 - ChattingResponse, ChattingRequest 생성 - SocketHandler에 예외발생시 로그파일로 만들게 변경 - ChattingMapper, ChattingService, ChattingMapper 클래스 생성을통한 채팅방 등록 및 찾기 구현 - room.jsp 에 uri 패턴 적용에 따른 변경

…-server into feature/30

f-lab-dev

전체적으로 확인 다시 해주시고 리팩토링 해주세요~

f-lab-dev · 2020-08-28T13:00:39Z

src/main/java/com/market/server/config/WebSocketConfig.java

+ @Bean
+ public TaskScheduler taskScheduler() {
+ ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler();
+ taskScheduler.setPoolSize(10);


나중에 가면 이 사이즈도 조절해야할 수 있습니다. 기억만 해주시고 이 사이즈를 바꾸면 어떤 영향이 있는지 확인해주세요~

poolsize가 커지면 그만큼 서버입장에서 메모리를 많이 필요로 합니다.
pool 모두를 사용하고 있다면 다음 테스크 스케줄러에 등록된 쓰레드 데이터 처리시 지연이 발생합니다.
한 테스크 스케줄이 pool을 반환해야 처리됩니다. 다시말해 순차적으로 처리합니다.
추가로 머신의 cpu갯수와 같은 최대 스레드 수만 등록할수 있습니다.

f-lab-dev · 2020-08-28T13:01:22Z

src/main/java/com/market/server/controller/ChattingController.java

+
+ static int roomNumber = 0;
+ private ChattingResponse chattingResponse;
+ List<RoomDTO> roomDTOList = new ArrayList<RoomDTO>();


이런쪽은 계속 수정해주세요~

f-lab-dev · 2020-08-28T13:01:39Z

src/main/java/com/market/server/controller/ChattingController.java

+ * @return
+ */
+ @GetMapping("/waiting-room")
+ public ModelAndView room() {


굳이 뷰는 필요 없어보입니다~

f-lab-dev · 2020-08-28T13:02:02Z

src/main/java/com/market/server/controller/ChattingController.java

+ mv.addObject("roomName", params.get("roomName"));
+ mv.addObject("roomNumber", params.get("roomNumber"));
+ mv.setViewName("chat");
+ }else {


컨벤션도 맞춰주세요

f-lab-dev · 2020-08-28T13:02:42Z

src/main/java/com/market/server/handler/SocketHandler.java

+public class SocketHandler extends TextWebSocketHandler {
+
+ private List<HashMap<String, Object>> rls = new ArrayList<>(); //웹소켓 세션을 담아둘 리스트 ---roomListSessions
+ private static final String FILE_UPLOAD_PATH = "C:/usedMarketServer/attachFile/";


이렇게 하면 서버에 배포되었을때 동작할까요?

상대경로로 파일명을 DownLoad하게 변경하겠습니다.

f-lab-dev · 2020-08-28T13:03:11Z

src/main/java/com/market/server/handler/SocketHandler.java

+ try {
+ wss.sendMessage(new TextMessage(obj.toJSONString()));
+ } catch (IOException e) {
+ e.printStackTrace();


이거는 의도하신 코드인가요?

알맞은 로그파일로 쓰게 변경하겠습니다.

- ChattingController에 chattingService 주입되게 변경 - roomDTOList를 DB에 저장한 데이터를 가져오게 변경

f-lab-dev · 2020-08-31T12:40:06Z

src/main/java/com/market/server/controller/ChattingController.java

- List<RoomDTO> roomDTOList = new ArrayList<RoomDTO>();
+ private final ChattingService chattingService;
+
+ @Autowired


생성자주입시에 이 어노테이션이 필요할까요?

생성자 주입시 단일 생성자(생성자가 2개 미만)인 경우에는 @Autowired 어노테이션 붙이지 않아도 주입됩니다.

f-lab-dev · 2020-08-31T12:40:45Z

src/main/java/com/market/server/controller/ChattingController.java

@@ -63,8 +72,9 @@ public ModelAndView room() {
 * @return
 */
 @GetMapping("/rooms")
- public @ResponseBody List<RoomDTO> getRoom(@RequestParam HashMap<Object, Object> params){
- return roomDTOList;
+ public @ResponseBody


@ResponseBody는 보통 메소드 위에 선언합니다. 이것도 컨벤션을 지키지 않은 것 같네요~

변경하겠습니다.

f-lab-dev · 2020-08-31T12:41:02Z

src/main/java/com/market/server/controller/ChattingController.java

- public @ResponseBody List<RoomDTO> getRoom(@RequestParam HashMap<Object, Object> params){
- return roomDTOList;
+ public @ResponseBody
+ List<RoomDTO> getRoom(@RequestParam HashMap<Object, Object> params, @RequestBody RoomDTO roomDTO) {


파라미터를 Map 타입으로 받으면 단점이 뭐가 있을까요?

현재 사용하지 않는 Key에 대해서 미리 Memory를 할당받아 사용하고 있는 단점이 있습니다.
load factor (key/메모리 size) 값을 넉넉히 할당받지 않으면 다시말해
load factor > 1 인 경우에는 Hash Collision이 발생합니다.

해결법으로는
Bucket Size를 단순히 2배로 늘린 후 Key값을 재배열합니다. 또는
충돌이 발생한 경우에 일정 Offset 간격으로 Bucket으로 접근하여 Search, Insert를 진행합니다.

유지보수성에서는 문제가 없을까요?

개발한 사람 이외의 사람들이 판단하기 어려운 점이 있습니다.
스프링에서는 커맨드 객체(Command Object)를 지원하여 HTTP에서 들어오는 각 속성 값들을
자동적으로 커맨드 객체에 바인딩하여 처리할 수 있게 변경가능합니다.
따라서 커맨드 객체(RoomDTO)를 이용하여@RequestMapping이나 getParameter로 처리하지
않아도 쉽게 처리 되게 커맨드 객체를 인자로 설정하겠습니다.
그에따라 훨씬 코드 양도 줄고, 가독성도 좋아지고 간편해집니다.
참고 https://engkimbs.tistory.com/693

유지보수성 측면에서 코드 가 명시적으로 보이기 위해
파라미터를 DTO형으로 변경하였습니다.

f-lab-dev · 2020-08-31T12:41:45Z

src/main/java/com/market/server/controller/ChattingController.java

@@ -77,7 +87,7 @@ public ModelAndView moveRoom(@RequestParam HashMap<Object, Object> params) {
 ModelAndView mv = new ModelAndView();
 int roomNumber = Integer.parseInt((String) params.get("roomNumber"));

- List<RoomDTO> new_list = roomDTOList.stream().filter(o->o.getRoomNumber()==roomNumber).collect(Collectors.toList());
+ List<RoomDTO> new_list = chattingService.getRooms(null).stream().filter(o->o.getRoomNumber()==roomNumber).collect(Collectors.toList());


chattingService.getRooms(null) 라는 시그니쳐를 보면 어떤 값이 리턴될지 예상이 가시나요?
차라리 getAllRooms()를 선언하시는게 나을 것 같습니다

변경했습니다.

f-lab-dev · 2020-08-31T12:42:38Z

src/main/resources/mybatis/mapper/chattingMapper.xml

+ `roomName`,
+ `type`
+ FROM `room`
+ LIMIT 100 OFFSET 0


OFFSET을 굳이 써주신 이유가 있으신가요? 그리고 이 쿼리는 딱 고정된 값만 가져올 것 같네요

추후에 읽은 채팅방은 표시되지 않게 하려고헀습니다.
고정값을 안쓰게 변경했습니다.

f-lab-dev · 2020-08-31T12:42:50Z

src/main/resources/mybatis/mapper/chattingMapper.xml

 </select>

+<!-- <select id="selectRooms" resultType="com.market.server.dto.RoomDTO">-->


주석은 올리지 않는게 좋습니다~ 차라리 지우고 올려주세요. 머지되는 코드는 최대한 완벽한 코드여야합니다. 이 피드백도 여러 번 드리는 것 같네요~

변경했습니다.

- application dev, release 버전 나누기 - Rooms정보 가져올시 order by 정보와 limit, offset 설정 파라미터 추가 - getLastRoomNumber 함수 추가 - ChattingController 쓰지않는 파라미터, 변수 제거

@responsebody

- 메소드명 명시적으로 변경 getRooms -> getAllRooms - 채팅중 파일 다운로드 디렉토리를 상대적으로 변경, 파일명은 처리 시간으로 변경 - 예외처리시 e.printStackTrace(); 부분을 log.error파일에 기록 - @responsebody 메소드 위로 수정 - taskScheduler 메소드 주석 추가

- gitignore에 attachFile 폴더 추가

f-lab-dev · 2020-09-01T13:38:02Z

src/main/java/com/market/server/controller/ChattingController.java

@@ -51,10 +42,11 @@ public ModelAndView room() {
 */
 @PostMapping("/rooms")
 public @ResponseBody
- List<RoomDTO> createRoom(@RequestParam HashMap<Object, Object> params, ChattingRequest chattingRequest) {
+ List<RoomDTO> createRoom(@RequestParam HashMap<String, String> params) {


유지보수성 측면에서 Map을 쓰는게 좋을지 고민해주세요~

유지보수성 측면에서 코드 가 명시적으로 보이기 위해
파라미터를 DTO형으로 변경하였습니다.

f-lab-dev · 2020-09-01T13:39:34Z

src/main/java/com/market/server/handler/SocketHandler.java

- File dir = new File(FILE_UPLOAD_PATH);
+
+ String fileName = new SimpleDateFormat("yyyyMMddHHmm'.jpg'").format(new Date());
+ File file = new File("");


안의 빈 문자열은 무슨 의미일까요?

의미없는 생성으로 System.getProperty("user.dir") 으로 대체하겠습니다.

f-lab-dev · 2020-09-01T13:40:22Z

src/main/java/com/market/server/handler/SocketHandler.java

- String fileName = "temp.jpg";
- File dir = new File(FILE_UPLOAD_PATH);
+
+ String fileName = new SimpleDateFormat("yyyyMMddHHmm'.jpg'").format(new Date());


전역 변수로 만들어놓고 돌려쓸 수 있지 않을까요? 혹은 날짜포맷 유틸로도 분리가 가능할것같네요.
대신 SimpleDateFormat 는 전역변수로 쓰지 못할 것 같습니다. 왜 그럴까요?

Synchronization
Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.

SimpleDateFormat 인스턴스는 필요한 경우 전역 상수로 선언 할 수 있지만
이 클래스는 스레드로부터 안전하지 않습니다. 여러 스레드가 동시에 액세스하는 경우 동기화해야합니다.

jdk 1.8 doc 참고

그럼 더 효율좋은 코드로 바꿔주세요~

f-lab-dev · 2020-09-01T13:40:54Z

src/main/java/com/market/server/handler/SocketHandler.java

+
+ String fileName = new SimpleDateFormat("yyyyMMddHHmm'.jpg'").format(new Date());
+ File file = new File("");
+ String fileDirectoryName = file.getAbsolutePath()+"\\usedMarketServer\\attachFile\\";


이렇게 하드코딩해도 될까요? 그리고 \\는 리눅스에서는 /가 될 것 같은데 어떻게 해야할까요?

File.separator 로 변경하였습니다.

f-lab-dev · 2020-09-01T13:41:05Z

src/main/java/com/market/server/handler/SocketHandler.java

@@ -81,7 +88,7 @@ public void handleBinaryMessage(WebSocketSession session, BinaryMessage message)
 byteBuffer.compact(); //파일을 복사한다.
 outChannel.write(byteBuffer); //파일을 쓴다.
 }catch(Exception e) {


여기도 컨벤션이 안지켜져 있네요~

변경했습니다.

f-lab-dev · 2020-09-01T13:41:26Z

src/main/java/com/market/server/handler/SocketHandler.java

@@ -91,7 +98,7 @@ public void handleBinaryMessage(WebSocketSession session, BinaryMessage message)
 outChannel.close();


try-with-resource 를 쓰면 어떨까요?

변경했습니다.

- Api 인자에 HashMap 형태를 DTO 타입으로 변경 - getLastRoomNumber 메소드에 NULL이 올수 있어서 예외처리, 반환 타입을 INT -> Interger로 변경 - spring배너 title 변경

- 무의미한 File 생성 제거 - System.getProperty("user.dir"), File.separator 를 이용한 프로젝트 상대경로 설정 - 첨부파일 경로 gitignore 추가

- DateUtil 생성 yyyyMMddHHmm 형태 문자열 반환, 확인 메서드 - FileOutputStream, FileChannel 사용시 try-with-resource 적용

f-lab-dev · 2020-09-04T12:11:22Z

src/main/java/com/market/server/controller/ChattingController.java

 if (roomName != null && !roomName.trim().equals("")) {
- int roomNumber = chattingService.getLastRoomNumber();
- roomDTO = RoomDTO.builder()
+ int roomNumber = 0;


아래에서 아무 조건없이 바로 할당하는데 여기에 0을 할당하신 이유가 있으신가요?

변경하겠습니다.

f-lab-dev · 2020-09-04T12:11:57Z

src/main/java/com/market/server/controller/ChattingController.java

- public List<RoomDTO> createRoom(@RequestParam HashMap<String, String> params) {
- String roomName = (String) params.get("roomName");
- RoomDTO roomDTO = null;
+ public List<RoomDTO> createRoom(RoomDTO roomDTO, Model model) {


model을 꼭 쓸 필요가 있을까요?

Controller에서 생성한 데이터를 담아서 View로 전달할 때 사용하는 객체로 roomDTO에 정보가 다있어서
삭제하겠습니다.

f-lab-dev · 2020-09-04T12:12:37Z

src/main/java/com/market/server/service/ChattingService.java

- public int getLastRoomNumber() {
- return chattingMapper.getLastRoomNumber();
+ public Integer getLastRoomNumber() {
+ Integer result = -2;


이거도 코드가 이상합니다~

변경하겠습니다.

f-lab-dev · 2020-09-04T12:12:47Z

src/main/java/com/market/server/service/ChattingService.java

@@ -28,14 +28,18 @@ public void register(RoomDTO roomDTO) {
 public List<RoomDTO> getAllRooms(RoomDTO roomDTO) {
 List<RoomDTO> roomDTOList = null;
 if (roomDTO == null)
- roomDTOList = chattingMapper.selectRooms("NEWEST", 20, 0);
+ roomDTOList = chattingMapper.selectRooms("NEWEST", 30, 0);


이런 값은 하드코딩보다는 config로 뺄 수 있지 않을까요?

ChattingService에 상수로 정의하겠습니다.

f-lab-dev · 2020-09-04T12:13:22Z

src/main/java/com/market/server/handler/SocketHandler.java

 String roomNumber = (String) rls.get(i).get("roomNumber"); //세션리스트의 저장된 방번호를 가져와서
- if(roomNumber.equals(rN)) { //같은값의 방이 존재한다면
+ if (roomNumber.equals(rN)) { //같은값의 방이 존재한다면


roomNumber가 null일 수도 있을까요?

roomNumber는 pk로 null일수 없습니다.

f-lab-dev · 2020-09-04T12:15:11Z

src/main/java/com/market/server/handler/SocketHandler.java

- log.error("파일 write 실패" , e);
- }finally {
+ } catch (Exception e) {
+ log.error("파일 write 실패", e);


로그만 찍고 아무 행위도 안하는게 의도하신게 맞을까요? 실패하면 실패했다고 알려줘야하지 않을까요?

IOException printStackTrace(); 추가했습니다.

f-lab-dev · 2020-09-04T12:16:21Z

src/main/java/com/market/server/handler/SocketHandler.java

- FileOutputStream out = null;
- FileChannel outChannel = null;
- try {
+ try (FileOutputStream out = new FileOutputStream(file, true); //생성을 위해 OutputStream을 연다.


여기도 괄호 뒤에 개행 한번 넣어주시면 가독성에 좋을 것 같습니다

변경했습니다.

f-lab-dev · 2020-09-04T12:17:28Z

src/main/java/com/market/server/utils/DateUtil.java

+ * @author topojs8
+ */
+ public static String getNowTimeToyyyyMMddHHmm(Date date, String fileType) {
+ return new SimpleDateFormat("yyyyMMddHHmm").format(date) + fileType;


이것도 결국 새로운 객체를 사용하는건 마찬가지같네요. 객체를 계속 생성하지 않을 수 있는 방법이 있지 않을까요?
이런 작아보이는 것들도 쌓여서 좋은 코드를 이루는 겁니다. 작은것도 도큐먼트를 꼼꼼히 읽어보고 다른게 뭐가 있을지 계속 찾아보시면서 개선해주세요~

f-lab-dev · 2020-09-04T12:18:01Z

src/main/java/com/market/server/utils/DateUtil.java

+ Date date = simpleDateFormat.parse(dateStr);
+ System.out.println("Successfully Parsed Date " + date);
+ } catch (ParseException e) {
+ System.out.println("ParseError " + e.getMessage());


System.out.println에 대한 것을 책에서 읽으신 것 같은데 코드에 반영이 안돼있네요.

변경했습니다.

f-lab-dev · 2020-09-04T12:18:07Z

src/main/java/com/market/server/utils/DateUtil.java

+ } catch (ParseException e) {
+ System.out.println("ParseError " + e.getMessage());
+ } catch (Exception e) {
+ e.printStackTrace();


여기도 마찬가지입니다

변경했습니다.

변경이 안되어있네요

f-lab-dev · 2020-09-04T12:19:53Z

리뷰를 하면서 느끼는게 리뷰에 나온 내용에 대해 알아보고 익히는게 아니라 그때그때 리뷰 내용을 반영하기만 하는 느낌입니다.
그렇게 하면 좋아보이는 코드는 만들 수 있겠지만 성장을 이뤄낼 순 없습니다. 특히 치명적인 코드가 몇개가 보이는데요. 꼼꼼함에 신경을 많이 쓰시는게 좋을 것 같습니다~

junshock5 · 2020-09-06T13:14:38Z

리뷰를 하면서 느끼는게 리뷰에 나온 내용에 대해 알아보고 익히는게 아니라 그때그때 리뷰 내용을 반영하기만 하는 느낌입니다.
그렇게 하면 좋아보이는 코드는 만들 수 있겠지만 성장을 이뤄낼 순 없습니다. 특히 치명적인 코드가 몇개가 보이는데요. 꼼꼼함에 신경을 많이 쓰시는게 좋을 것 같습니다~
-> 치명적인 코드가 어떤건지 명시해주시면 조금더 신경쓰겠습니다.

- 파일명 생성하는 DateFormat을 ThreadLocal을 사용하여 재사용하게 변경 - ChattingService에 채팅목록 검색시 Default설정 상수화 - SocketHandler에 파일 인덱스, 파일 디렉토리명 상수화, 예외처리 에러 추가 - ChattingController 필요없는 로컬변수, model 파라미터 제거

f-lab-dev · 2020-09-08T12:45:25Z

System.out.println가 치명적인 것 중에 하나입니다~ 쓰면 안돼는 메소드죠

f-lab-dev · 2020-09-08T12:46:01Z

src/main/java/com/market/server/handler/SocketHandler.java

 ) {
 byteBuffer.flip(); //byteBuffer를 읽기 위해 세팅
 byteBuffer.compact(); //파일을 복사한다.
 outChannel.write(byteBuffer); //파일을 쓴다.
- } catch (Exception e) {
+ } catch (IOException e) {
+ e.printStackTrace();


여기도 쓰면 안돼는 메소드를 쓰셨네요

e.getMessage(), e.toString()

에러 메세지만 간략하게 보여준다.

두 개의 차이점은 그냥 java exception이냐 보여주는 차이. 찍어보면 알 수 있다.

e.printStackTrace()

모두가 아는.. 처음 호출한데서부터 에러 발생한 끝까지 들어간 내용 그대로 보여준다. 말 그대로 보면, stack을 trace 했으니. 처음 호출에서부터 차곡차곡 쌓인 모든 함수들을 그대로 뿌려준다.

e.getMessage(), e.toString() 를 사용해도 될까요??

f-lab-dev · 2020-09-08T12:46:46Z

src/main/java/com/market/server/utils/DateUtil.java

+ public static final String getNowTimeToyyyyMMddHHmm(Date date, String fileType) {
+ SimpleDateFormat sdf = tl.get();
+ if(sdf == null) {
+ sdf = new SimpleDateFormat("yyyyMMddHHmm");


있는지 검사하고

없으면 새로 생성한다

이거는 thread safe 한가요? 그리고 스레드로컬 사용법에 대해 잘못 이해하고 계신듯 하네요~

로컬변수여서 thread safe한 줄 알고 있는데 아닐까요??

f-lab-dev · 2020-09-08T12:47:07Z

src/main/java/com/market/server/utils/DateUtil.java

@@ -28,9 +37,10 @@ private static void parseDate(String dateStr) {
 try {
 SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyyMMddHHmm");
 Date date = simpleDateFormat.parse(dateStr);
+ log.info("Successfully Parsed Date " + date);


로그 포맷팅을 사용해주세요

변경했습니다.

f-lab-dev · 2020-09-08T12:47:16Z

src/main/java/com/market/server/utils/DateUtil.java

+ } catch (ParseException e) {
+ System.out.println("ParseError " + e.getMessage());
+ } catch (Exception e) {
+ e.printStackTrace();


변경이 안되어있네요

- System.out.println문 삭제 - Exception 문 삭제 - ThreadLocal문 변경 - 메인 어플리케이션 명 변경

…-server into feature/30

f-lab-dev · 2020-09-12T15:46:15Z

src/main/java/com/market/server/utils/DateUtil.java

@@ -37,12 +37,9 @@ private static void parseDate(String dateStr) {
 try {
 SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyyMMddHHmm");
 Date date = simpleDateFormat.parse(dateStr);
- log.info("Successfully Parsed Date " + date);
- System.out.println("Successfully Parsed Date " + date);
+ log.info("Successfully Parsed Date " , date);


이건 굳이 찍어야할 로그일까요? 불필요한 로그를 계속 찍으면 분석할 의미가 없는 로그가 엄청 늘어나게 됩니다

삭제하였습니다.

f-lab-dev

전체적으로 다시 돌아보니 리뷰를 드렸음에도 리뷰를 드린 곳에만 반영이 되고 다른 곳에는 기존에 공부했던 내용들이 반영되지 않은 경우가 많네요. 그리고 기본적인 카멜케이스 같은 것조차 지켜지지 않은 곳이 있어 리뷰의 의미가 없어보여서 리뷰를 중단하고 기존에 남기던 것 몇개만 남깁니다.

코드리뷰는 당장의 코드의 허술함만 땜질하는 것뿐이 아니라 지식을 습득하여 미래에 더 나은 코드를 짜기 위한 목적도 있습니다. 그리고 그 지식을 습득할 수 있는 레퍼런스도 드렸었고요.

지금 이 PR은 아무 코드나 가져온 다음 리뷰만 적용해서 당장 포트폴리오로 쓸만한 적절하게 그럴듯해보이는 코드를 만들려는 목적으로 보이기도 합니다. 계속 말씀드리는 것이지만 단순하게 웹소켓을 썼다라는 것만으로는 원하시는 기업의 다른 면접들에 가서도 커버를 못칠겁니다.

여태까지 습득한 지식들이 코드에 녹아있어야하는데 여기 작성된 코드들을 보면 여태까지 진행한 멘토링 몇 개월동안 공부한게 물거품으로 보일 정도로 너무 허술합니다. 17개밖에 안돼는 파일에서 리뷰를 조금 더 달면 100개를 돌파할 것 같고요.

추후 멘토링을 그만하시더라도 꼭 이해해두셔야하는 것은 단순히 "~ 기술을 사용해봤다", "~를 경험했다"만으로는 어필이 안됩니다. 얼마나 "잘" 사용하려했는지가 중요한데 여기는 기본적인 자바코드부터가 "잘" 사용되고 있지 않네요.

클라우드 제품 사용 경험을 위해 무료크레딧이 제공되는 애저나 네이버클라우드를 권했었을때도 별다른 이유나 코멘트 없이 AWS를 사용하시기도 했고, 블로그글 작성 때도 문제가 있었죠. 물론 제가 말씀드린 내용이 합리적이지 않다고 판단하셨을수도 있지만 그렇다면 합리적이지 않은 이유를 들며 더 합리적인 방향으로 가기 위해 커뮤니케이션을 시도하셨어야합니다.

어느정도 경력이 있는 사람은 개발습관과 생각이 박혀버려서 그걸 바꾸기가 쉽지 않습니다. 그 한계를 벗어나셔야 개발자로써 한걸음 더 나아가실 수 있을 것 같네요.

앞으로 어떻게 할지는 고민을 좀 해보고 추후에 얘기를 좀 나눠보면 좋겠네요

f-lab-dev · 2020-09-14T14:26:21Z

src/main/resources/application-dev.properties

@@ -17,6 +17,7 @@ market.server.redis.password=
 # expire
 # millisecond 600����� �� 10�ð�


뭔가 글자가 깨진것 같네요

f-lab-dev · 2020-09-14T14:26:31Z

src/main/resources/application-dev.properties

@@ -17,6 +17,7 @@ market.server.redis.password=
 # expire
 # millisecond 600����� �� 10�ð�
 expire.defaultTime=36288000
+expire.products=5


이거는 밀리초인가요?

5000으로 변경했습니다.

f-lab-dev · 2020-09-14T14:26:53Z

src/main/resources/application.properties

@@ -1,5 +1,5 @@
 # Server
-server.port=8888
+server.port=9000


이제는 로컬용 포트변경코드는 지우는게 낫지 않을까요?

f-lab-dev · 2020-09-14T14:28:36Z

src/main/java/com/market/server/controller/ChattingController.java

+ String roomName = roomDTO.getRoomName();
+ RoomDTO room = null;
+
+ if (roomName != null && !roomName.trim().equals("")) {


roomName이 null이면 NPE가 날 것 같네요

f-lab-dev · 2020-09-14T14:29:51Z

src/main/java/com/market/server/controller/ChattingController.java

+ RoomDTO room = null;
+
+ if (roomName != null && !roomName.trim().equals("")) {
+ int roomNumber = chattingService.getLastRoomNumber();


이것도 동시성문제가 있어보이네요. 이 메소드는 호출할때마다 다른값을 반환하나요? 혹은 같은 값을 반환할수도 있나요?

RoomDTO가 정상적으로 등록되지 않았다면 같은값을 반환할 수도 있습니다.

f-lab-dev · 2020-09-14T14:30:55Z

src/main/java/com/market/server/controller/ChattingController.java

+ ModelAndView mv = new ModelAndView();
+ int roomNumber = roomDTO.getRoomNumber();
+
+ List<RoomDTO> new_list = chattingService.getAllRooms(null).stream().filter(o -> o.getRoomNumber() == roomNumber).collect(Collectors.toList());


여기는 카멜케이스가 지켜지지 않았네요

- createRoom NPE 구문 추가 - createRoom convention 적용 - 로컬포트 삭제 - expire.products=5000 5초로 변경

junshock5 · 2020-09-15T13:30:23Z

우선 목요일 코딩테스트, 금요일 과제 이후에
기존에 리뷰주셨던 것들을 복습하면서
전체적으로 코드들을 다시 리뷰하겠습니다.

junshock5 added 2 commits August 24, 2020 19:11

#30 웹소켓을 이용한 1:1 채팅 구현

f455586

- WebSocket 의존성 추가 - WebSocketConfig 추가 - json 의존성 추가 - SocketHandler class 추가 - Chatting Controller 추가

#30 웹소켓을 이용한 1:1 채팅 구현

5fabb1d

- 채팅 jsp 처리를 위한 servlet추가 - ModelAndView 사용 및 chat.jsp 추가 - spring.mvc.view 설정

junshock5 requested a review from f-lab-dev August 24, 2020 10:58

junshock5 self-assigned this Aug 24, 2020

junshock5 added the review wanted 리뷰 부탁드립니다. label Aug 24, 2020

junshock5 added 3 commits August 25, 2020 12:43

#30 웹소켓을 이용한 1:1 채팅 구현

bf32376

- json-simple 의존성 추가 - sessionID값을 기준으로 채팅 상대방과 분리되게 JSP 변경 - 채팅 message jsonToObjectParser 함수로 파싱 되게 변경

#30 웹소켓을 이용한 1:1 채팅 구현

c19f1ba

- messageType이 BinaryMessage인 데이터 디렉터리에 저장 및 발송 - chat.jsp에 웹소켓을 통해 fileSend함수 생성

junshock5 added 5 commits August 28, 2020 01:29

Merge branch 'develop' into feature/30

cfdde06

Merge branch 'feature/29' into feature/30

88e212c

Merge branch 'feature/30' of https://github.com/f-lab-edu/used-market…

f56371c

…-server into feature/30

f-lab-dev reviewed Aug 28, 2020

View reviewed changes

#30 웹소켓을 이용한 1:1 채팅 구현

fc9f045

- ChattingController에 chattingService 주입되게 변경 - roomDTOList를 DB에 저장한 데이터를 가져오게 변경

f-lab-dev reviewed Aug 31, 2020

View reviewed changes

junshock5 added 4 commits August 31, 2020 22:14

#30 웹소켓을 이용한 1:1 채팅 구현

bd8c3b2

- application dev, release 버전 나누기 - Rooms정보 가져올시 order by 정보와 limit, offset 설정 파라미터 추가 - getLastRoomNumber 함수 추가 - ChattingController 쓰지않는 파라미터, 변수 제거

#30 웹소켓을 이용한 1:1 채팅 구현

c5a76b7

- gitignore에 attachFile 폴더 추가

attachFile 잘못올라간거 삭제

6e23408

f-lab-dev reviewed Sep 1, 2020

View reviewed changes

junshock5 added 3 commits September 4, 2020 00:16

#30 웹소켓을 이용한 1:1 채팅 구현

f4f1fad

- Api 인자에 HashMap 형태를 DTO 타입으로 변경 - getLastRoomNumber 메소드에 NULL이 올수 있어서 예외처리, 반환 타입을 INT -> Interger로 변경 - spring배너 title 변경

#30 웹소켓을 이용한 1:1 채팅 구현

8d887b5

- 무의미한 File 생성 제거 - System.getProperty("user.dir"), File.separator 를 이용한 프로젝트 상대경로 설정 - 첨부파일 경로 gitignore 추가

#30 웹소켓을 이용한 1:1 채팅 구현

5fb8d10

- DateUtil 생성 yyyyMMddHHmm 형태 문자열 반환, 확인 메서드 - FileOutputStream, FileChannel 사용시 try-with-resource 적용

f-lab-dev reviewed Sep 4, 2020

View reviewed changes

junshock5 added 2 commits September 6, 2020 22:58

Merge branch 'feature/29' into feature/30

825152d

f-lab-dev reviewed Sep 8, 2020

View reviewed changes

junshock5 added 2 commits September 12, 2020 23:27

#30 웹소켓을 이용한 1:1 채팅 구현

e4e31df

- System.out.println문 삭제 - Exception 문 삭제 - ThreadLocal문 변경 - 메인 어플리케이션 명 변경

Merge branch 'feature/30' of https://github.com/f-lab-edu/used-market…

f1d226c

…-server into feature/30

f-lab-dev reviewed Sep 12, 2020

View reviewed changes

junshock5 added 2 commits September 13, 2020 01:00

캐싱 만료시간 추가 , DateUtil 필요없는 로그 삭제

05eba04

Merge branch 'feature/29' into feature/30

fdb1bf9

f-lab-dev reviewed Sep 14, 2020

View reviewed changes

#30 웹소켓을 이용한 1:1 채팅 구현

9717040

- createRoom NPE 구문 추가 - createRoom convention 적용 - 로컬포트 삭제 - expire.products=5000 5초로 변경

		</select>

		<!-- <select id="selectRooms" resultType="com.market.server.dto.RoomDTO">-->

		@@ -91,7 +98,7 @@ public void handleBinaryMessage(WebSocketSession session, BinaryMessage message)
		outChannel.close();

		@@ -17,6 +17,7 @@ market.server.redis.password=
		# expire
		# millisecond 600�� 10�ð�

#30 웹소켓을 이용한 1:1 채팅 구현 #32

Are you sure you want to change the base?

#30 웹소켓을 이용한 1:1 채팅 구현 #32

Conversation

junshock5 commented Aug 24, 2020

f-lab-dev commented Aug 25, 2020

f-lab-dev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junshock5 Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junshock5 Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

junshock5 Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junshock5 Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f-lab-dev commented Sep 4, 2020

junshock5 commented Sep 6, 2020

f-lab-dev commented Sep 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junshock5 Sep 1, 2020 •

edited

Loading

junshock5 Sep 2, 2020 •

edited

Loading

junshock5 Sep 3, 2020 •

edited

Loading

junshock5 Sep 3, 2020 •

edited

Loading

f-lab-dev left a comment •

edited

Loading