[5기 최인준] Shorten-URL 과제 제출합니다 #60
Conversation
|
|
||
| public abstract class CommonException extends RuntimeException { | ||
|
|
||
| public abstract String getMessage(); |
There was a problem hiding this comment.
RuntimeException 안에 getMessage() 메소드가 있는거로아는데 getMessage() 메소드를 따로 선언하신 이유가 있으실까요?
There was a problem hiding this comment.
getMessage()를 해당 클래스의 추상 메소드로 두고 싶어서 이런식으로 했는데 굳이 필요하지 않은 부분일까요?
| @Override | ||
| public String encode(Long id) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| while (id > 0) { | ||
| log.info("id % radix = {}", (int) (id % RADIX)); | ||
| sb.append(BASE62_CHARACTERS.charAt((int) (id % RADIX))); | ||
| id /= RADIX; | ||
| } | ||
| return sb.toString(); |
There was a problem hiding this comment.
이렇게 구현되면 글자수가 1개부터 시작할 것 같은데, 의도하신 걸까요?
There was a problem hiding this comment.
흔히 있는 인코딩 알고리즘을 써서 그렇게 됐습니다.. 인코딩 알고리즘에 대해 더 자세히 알아봐야 할 것 같습니다!
| findUrl.get().getHits()); // 단축 URL, 누적 요청횟수 리턴 | ||
| } | ||
|
|
||
| Url url = new Url(longUrl, "", 0); |
There was a problem hiding this comment.
너무 부자연스러운 생성자 호출같은데, 이 과정에서 필요한 생성 메소드나 생성자를 만드는건 어떨까요?
| public Url(String longUrl, String shortUrl, int hits) { | ||
| this.longUrl = longUrl; | ||
| this.shortUrl = shortUrl; | ||
| this.hits = hits; | ||
| } |
There was a problem hiding this comment.
처음에 생성되는 Url 은 hits 가 0 으로 초기화되는거로 알고 있는데, 파라미터로 받는거보단 내부적으로 0 으로 초기화하는건 어떨까요?
There was a problem hiding this comment.
무조건 0으로 생성되니까 그렇게 하는게 맞는 것 같습니다. 이 리뷰 주신 부분 위 리뷰랑 엮어서 반영하겠습니다!
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| Url url = (Url) o; | ||
| return hits == url.hits && Objects.equals(id, url.id) && Objects.equals(longUrl, | ||
| url.longUrl) && Objects.equals(shortUrl, url.shortUrl); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(id, longUrl, shortUrl, hits); | ||
| } |
There was a problem hiding this comment.
해당 클래스에 equalsAndHashCode 를 구현하신 이유가 궁금합니다 :)
There was a problem hiding this comment.
테스트 할 때 편했을 때가 있어 구현했는데 테스트를 짜지않아.. 없어도 될 것 같습니다
There was a problem hiding this comment.
UrlEncoder 네이밍이고 Url 도메인 안에서만 사용되는 것 같은데 common 으로 분리하신 이유가 있으실까요?
|
|
||
| public Url(String longUrl) { | ||
| this.longUrl = longUrl; | ||
| this.shortUrl = ""; |
There was a problem hiding this comment.
생성할 때 빈 값으로 생성 후에 제대로 된 값으로 변경하지말고 생성 시에 제대로 된 값으로 넣어주면 좋을 것 같습니다.
생성된 이후엔 따로 정상적인 값이 들어있는지 확인할 필요가 없으면 좋은 것 같아요!
| @Column(name = "short_url") | ||
| private String shortUrl; | ||
|
|
||
| @Column(name = "request_hits") |
There was a problem hiding this comment.
크게 중요한건 아닌데 양수만 가능한 필드면 @Column(columnDefinition = "UNSIGNED INT(11)")로 지정하는 것도 괜찮습니다.
📌 과제 설명
원본 URL을 입력받아 Short Url을 생성해주고 Short Url을 통해 원본 Url에 접근할 수 있는 서비스입니다.
👩💻 요구 사항과 구현 내용
요구사항
구현 내용
✅ PR 포인트 & 궁금한 점