feat : 회원가입 및 로그인 기능 구현(first Version) #1
Conversation
| } | ||
|
|
||
| @Bean | ||
| public MemberService memberService(){ |
There was a problem hiding this comment.
컴포넌트 스캔을 활용하는게 더 나을 것 같습니다.
MemberService와 MemberRepository 는 @Service 와 @Repository를 활용하여 빈으로 등록되게 하도록 해 주세요.
(+생성자 주입)
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class LoginServiceImpl { |
There was a problem hiding this comment.
�Impl 수식어는 일반적으로 인터페이스가 존재하고, 해당 인터페이스의 구현체가 존재하는 경우에 사용합니다.
현재 상황의 경우, 딱히 인터페이스가 있는게 아니므로 Impl 이라는 이름을 제거해 주세요.
(꼭 인터페이스를 만들 필요는 없고, 오히려 구현체가 하나인 경우엔 오히려 없는게 낫습니다.)
|
|
||
| @Controller | ||
| @RequiredArgsConstructor | ||
| public class LoginController { |
There was a problem hiding this comment.
굳이 /members/를 명시하는 것 보단, 클래스 최상단에 @RequestMapping("/members") 를 박아버리는게 더 낫습니다.
이후 다른 기능을 추가해도, 어떤 경로에 어떤 API가 있는지 확인할 수 있으니까요.
| @@ -0,0 +1,96 @@ | |||
| package org.poolc.controller; | |||
|
|
|||
| import com.sun.xml.bind.v2.TODO; | |||
There was a problem hiding this comment.
잘못 import 된 것 같네요.
IntelliJ에서 Save Action 플러그인을 설치하여 저장시 자동으로 사용하지 않는 import 를 제거하도록 하거나,
Optimize Import 기능 (Mac 기준 Ctrl + Option + O) 을 사용해서 제거하도록 해 주세요.
| private final MemberService memberService; | ||
|
|
||
| @Autowired | ||
| public MemberController(MemberService memberService){ |
There was a problem hiding this comment.
생성자가 1개인 경우엔 @Autowired가 필요하지 않습니다.
또한, 다른 빈들은 @RequiredArgConstructor를 사용하고 있는데 여긴 수동으로 만들고 있네요.
통일하는게 좋을 것 같습니다.
|
|
||
| public String create(@Valid @ModelAttribute("member") Member member, BindingResult bindingResult){ | ||
| // 회원가입 정보가 valid하지않은 경우(채우지 않은 칸 존재) -> 다시 회원 가입 창으로 redirection | ||
| if(bindingResult.hasErrors()){ |
There was a problem hiding this comment.
회원 체크 로직이 생각보다 많이 등장하는 것 같은데, 이걸 묶을 수 있는 방법이 없을지 찾아보면 좋을 것 같습니다.
(힌트: HttpServletRequest의 동작 과정을 알아보거나, AOP에 대해 알아보는 것을 권장합니다.)
|
|
||
| @Controller | ||
| @RequiredArgsConstructor | ||
| public class LoginController { |
There was a problem hiding this comment.
로그인은 member/login이 아닌, 별도의 경로를 타도록 하는 것이 이상적인 설계일 것 같습니다.
| //저장공간 ->key:회원id/value:Member static으로 공유변수일때는 동시성 문제때문에 concurrnetHashmap써야함 | ||
| private static Map<Long,Member> store = new HashMap<>(); | ||
| //키값을 생성해주는얘, 애도 동시성 문제때문에 atom long등을 해주어야함 | ||
| private static long sequence = 0L; |
There was a problem hiding this comment.
atom long을 고민하다가 그냥 long으로 결정하신 것 같은데, AtomicLong 이라는 클래스를 사용해 보세요.
|
|
||
|
|
||
|
|
||
| public class MemberServiceImpl implements MemberService{ |
| implementation 'org.apache.commons:commons-collections4:4.4' | ||
| implementation 'org.eclipse.persistence:javax.persistence:2.2.1' |
|
넵 멘트 감사합니다! 주신 조언들 참고해서 코드 리팩토링 해보겠습니다! |
웹페이지 동작을 위한 MVC base 구현
Http세션 방식으로 동작하는 로그인 및 로그아웃
H2기반 DB스키마 설계
비밀번호 암호화를 위한 Spring Security 사용
기능별 단위테스트 작성