[Review] 5단계 - 동적 HTML (2022/04/03)

 

[케이 & Shine] 5단계 - 동적 HTML by zbqmgldjfh · Pull Request #65 · codesquad-members-2022/java-was

1. 코드 리뷰

이번 리뷰는 Roach가 해주셨다! 감사합니다 로치!!

 

1. JSessionId 구현

비슷하게 나마 sessionId를 유사하게 구현하려 노력했던 부분이다.

save를 하면서 저장을 하고, 반환값으로 sessionId를 반환하도록 하였는데, 이부분이 잘 구현된것 같아 다행이다.

 

2. Session에 저장할때 Object로 받기!

생각해보니 좋은 요청인것 같다. 우리팀은 단순하게 요구조건만 생각하다보니 User만 저장할 생각을 했다.

Object를 받도록 변경하는것이 확실하게 더 좋을것 같다.

코드를 다음과 같이 변경하였다.

public class Session {
    public static final String SESSION_ID = "sessionId";
    private static Map<String, Object> sessionDB = new ConcurrentHashMap<>();

    public static String save(Object value) {
        String sessionId = UUID.randomUUID().toString();
        sessionDB.put(sessionId, value);
        return sessionId;
    }
    // 생략...
}

 

3. Session에 부합하는 기능만 추가하기

생각해보니 그렇다.

Session에 왜 isLoginUser가 있어야 하는가?

 

물론 처음에는 OOP 적으로 객체에게 질문을 날리기 위해 만든 함수이기는 하지만, 사실 Session에게 직접 로그인 하였는지 확인하는 메서드를 날리기 보다는, 기존의 실제 서버의 session 처럼 해당 key값을 갖는 value를 반환시켜 이를 활용하는것이 더 좋을것 같다.

사용하는 쪽에서는 session.getattribute 와 같이 사용하도록 수정했다!

 

따라서 Session 쪽에 다음과 같은 메서드를 구현하였다.

public static Object getAttribute(String id) {
    return sessionDB.get(id);
}

원래 Optional로 반환할까 하다가... 그냥 순수 null도 반환할수 있도록 두었다.

 

4. Collection을 반환하는 이유

원래는 List와 같은 종류로 casting 해보려 했는데, 이게 DataBase가 ConcurrentHashMap 을 사용하고 있어서 values()로 반환하는 기본값이 Collection 이였습니다.

따로 stream을 처리하여 collect을 적용하여 List로 반활할수도 있어지만, 우선을 기본값을 사용하기로 했었다!!

 

5. Depth 1로 줄이기

depth 2로 if문을 2번 들어갈 필요가 없었다!

나도 지금 보니 왜 저렇게 둔지 모르겠다!! if 문안에서 and 조건으로 depth1로 처리되도록 변경하였다.

 

6. 매직넘버 지우기

해당 리뷰는 저번 시간에도 받았던 리뷰인데.... 왜 고치지를 못하는것인가 나는.... 매직넘버좀 그만 쓰자!! 습관화 시키자!!

다음과 같이 변경하게 되었다.

if (splits.length > MIN_LENGTH) {
    return splits[SECOND_SESSION].substring(SESSEION_ID_BEGIN_INDEX);
}