-
Notifications
You must be signed in to change notification settings - Fork 0
[Assignment] 민혜 - 양방향 연결 리스트 구현 #20
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
base: main
Are you sure you want to change the base?
The head ref may contain hidden characters: "7/\uBBFC\uD61C/\uC591\uBC29\uD5A5\uC5F0\uACB0\uB9AC\uC2A4\uD2B8"
Conversation
|
어썸합니다~~ |
JengYoung
left a comment
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.
미네님~ 잘 지내시죠!
에구 제가 오늘 노이로제가 와서 늦게나마 리뷰 달아드렸어요.
가끔씩 개발일지 잘 염탐하고 있답니다. 짧은 기간 동안 정말 많이 성장하신 게 느껴졌어요! 🙆🏻
바쁜 와중에 좋은 과제 올려주셔서 감사드려요. 몇 개 예상되는 엣지케이스 남겼어요!
조만간 여유가 생기면, 자주 새로운 과제로 찾아뵙겠읍니다 😉 같이 성장해요 🏃
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.
const로 해주시는 게 안전할 것 같아요 :)
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.
궁금한 게 있어요!
push에서는 하나도 없을 때 this.tail과 this.head를 할당해주었어요.
remove를 사용할 때, 노드가 단 하나만 존재한다면 취약점이 발생하지 않을까요?
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.
노드가 하나만 존재할 경우는 shift나 pop 메서드 호출을 하도록 바꿔볼게요!
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.
index는 number 타입일텐데, 이 코드가 어떤 의미를 가지려 했는지 궁금합니다!
this.length였을까요?
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.
네 오타인 것 같아요 ㅎㅎ..
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.
새로운 노드 -> 이전의 노드와는 호환이 잘 되는 것을 이해했어요.
하지만 양방향 연결 리스트에서는 새로운 노드와 다음 노드 역시 잘 이어져야 할 것 같아요.
현재의 로직에서는 다음 노드의 이전 노드가 아직 previousNode를 가리키고 있지 않을까 싶어요!
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.
헉 그렇네용..! afterNode를 만들어서 새로운 노드랑 연결해볼게요!
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.
이 코드가 반복해서 쓰이는 것 같아요.
목적이 워낙 뚜렷하고 쉽게 바뀌지 않는 로직일 것 같아서 따로 메서드를 만들어서 재사용해주는 것도 좋아보입니다!
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.
오오 생각치 못했던 부분이네요 ..! 따로 메서드를 만들어보겠습니다 ㅎㅎ
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.
해당 코드가 서버에서 응답할 때 호출되는 메서드이며, get이 다른 사용자에게 큰 영향을 줄 수 있다 가정해봅시다!
이때 사용자가 악의를 갖고 -1을 넣는다면 무슨 일이 발생할까요!
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.
머리 노드가 나오겠네요 ㅠㅠ index 값을 걸러주는 조건문을 넣어보겠습니다!
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.
앗 요건 제가 잘못 생각했던 부분이었네요 😭 while문이 무한루프 빠질 줄 알았는데, 생각해보니 바로 머리노드가 나오겠네요.
찰떡같이 받아들여주셔서 감사합니다...🙆🏻
하지만 여전히 취약점은 존재하는 것 같아요. 인덱스를 Number.MAX_SAFE_INTEGER 로 준다면 에러가 나올 수 있습니다 :)
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.
쓸까말까 고민하다 간만에 추억을 되살릴 겸 적읍니다.
EOL은 지키는 게 항상 좋읍니다 😉 (저도 안 지키지만...)
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.
테스트코드라고 prettier 적용하는걸 까먹었나 봅니다.. 😂 EOL ! EOL !
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.
만약 아무것도 없을 때 shift를 한다면 어떨까요?
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.
오류가 날것 같습니다 ㅠㅠ 아무것도 없을 때 false를 반환하는 조건문 추가해야겠네요..!
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.
afterNode가 null일 수도 있지 않을까요!
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.
afterNode가 null인 경우는 pop 메서드를 사용하도록 코드를 추가해야겠네요!
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.
리스트의 길이가 0~1개일 때 모두 문제가 발생할 수 있지 않을까요?
JengYoung
left a comment
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.
요건 다시 훑어볼 때 추가로! (호다닥)
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.
요기도 const!
himyne
left a comment
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.
기능 구현에 치중해서 예외 사항을 너무 지나치고 코드를 짠 것 같네요😥 여러가지 경우의 수를 생각하면서 코드를 짜는 연습을 해야겠어요! 세세한 리뷰 감사해요 🙏🏻
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.
네 오타인 것 같아요 ㅎㅎ..
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.
헉 그렇네용..! afterNode를 만들어서 새로운 노드랑 연결해볼게요!
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.
머리 노드가 나오겠네요 ㅠㅠ index 값을 걸러주는 조건문을 넣어보겠습니다!
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.
오류가 날것 같습니다 ㅠㅠ 아무것도 없을 때 false를 반환하는 조건문 추가해야겠네요..!
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.
넵 여기도 if문 추가할게요!
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.
afterNode가 null인 경우는 pop 메서드를 사용하도록 코드를 추가해야겠네요!
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.
노드가 하나만 존재할 경우는 shift나 pop 메서드 호출을 하도록 바꿔볼게요!
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.
오오 생각치 못했던 부분이네요 ..! 따로 메서드를 만들어보겠습니다 ㅎㅎ
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.
테스트코드라고 prettier 적용하는걸 까먹었나 봅니다.. 😂 EOL ! EOL !
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.
요것도 조건문으로 예외 처리 해야겠군요 ✨
📄 설명
양방향 연결리스트를 구현했습니다 !
테스트케이스는 jest를 이용하여 작성했고 주어진 테스트 코드는 모두 통과했습니다 🧪
이슈에 있던 체크리스트를 모두 구현하고 추가 기능 구현도 해봤어요!
remove 메서드에 인덱스를 인자로 받아 해당 인덱스 노드를 삭제하도록 코드를 짜고 있었는데요.
내부가 너무 복잡해져서 머리랑 꼬리 노드는 따로 처리해주어야 할 것 같아 pop메서드와 shift 메서드를 추가했습니다!
🔗 관련 이슈
#17
👀 논의해 볼 사항
🔑 참고할 만한 소스
유데미 알고리즘 강의에서 단방향 연결 리스트에 대한 내용을 먼저 듣고 이를 참고하여 양방향 연결리스트도 구현해봤습니다.
유료지만 강의 링크와 단방향 연결 리스트를 정리한 노션 링크 첨부할게요!
유데미 강의
노션 링크