Skip to content

Conversation

@Deocksoo
Copy link

No description provided.

kwangilcho and others added 16 commits July 23, 2018 00:44
Linked list는 크기가 정해져있지 않아서 새로운 element가 추가될 때마다 새로운
Node를 하나씩 만들어 linked list에 추가해주어야 하므로, 동적 할당을 이용한다.
주어진 과제에서 Linked list에 들어갈 숫자들이 양의 정수이므로, element들이 들어가는 node들과 구분하기
위하여 header node의 element를 -1로 초기화한다.
이번 과제에서 구현해야할 것은 1) Insert, 2) delete, 3) Find the previous, 4) show the list이다.
위의 사항들 중 1~2번 항목을 보다 수월하게 구현하기 위해서 FindNode 함수를 먼저 구현하였다.
Delete 함수를 보다 수월하게 구현하기 위하여 추가하는 함수이다.  Node 하나를 지우면
지운 Node의 previous node와 next node를 연결해주어야 하는데, 이 때 previous node를
찾아주기 위해 필요한 함수이다.
Linked list에 없는 key element의 previous element를 찾는 경우에, NULL이 아닌 마지막 element를
return 하게 되는 오류가 있어 이를 수정한다.
index 변수는 element들의 index도 함께 출력해주기 위해 print 되고 있는 element의 index를
계산하기 위한 변수이다. 

IsEmpty 함수는 linked list가 비어있는지 여부를 판단해주는 함수로, list가 비어있으면 1을 return
한다. 출력할 element가 없는 경우에 대한 예외 처리를 보다 가독성 있게 하기 위하여 앞서 정의하였다.
어떤 node가 delete되면 그 node를 가리키는 pointer는 delete된 node의 next node를 가리키게
되는데, 그러면 delete된 node를 free 시켜주지 않으면 사용되지 않는 메모리를 낭비하게 된다.
그래서 deleted node를 free 시켜주기 위해 temp 포인터변수를 따로 사용하였다.
Input file 한 줄을 buffer로 입력받고, buffer의 첫 번째 문자로 올 수 있는 i, d, f, p 네가지 명령에
대하여 각각 Insert, Delete, Find, Print 의 동작을 수행하도록 if문을 구성하였다.
Input file 한 줄을 buffer로 입력받고, buffer의 첫 번째 문자로 올 수 있는 i, d, f, p 네가지 명령에
대하여 각각 Insert, Delete, Find, Print 의 동작을 수행하도록 if문을 구성하였다.
Element들을 넣을 linked list를 만들고, input line을 입력받을 buffer를 array로 만든다.
buffer의 크기는 input line의 길이보다 충분히 크도록 임의로 설정하였다.
Copy link
Contributor

@kwangilcho kwangilcho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

훌륭하게 잘 구현하셨네요. 👍

  1. 커밋수가 조금 과하게 많기는 하지만, 의식적으로 커밋을 찍는 습관을 기르는 동안은 이렇게 연습하셔도 괜찮습니다. 혹은 현재 커밋의 수준으로 3~4개 정도를 묶어서 하나의 커밋으로 만드는 것도 괜찮을 것 같네요.

  2. 코드에서 불필요한 공백들이 다소 보였습니다. 문단이 확실히 의미적으로 구분될 때에만 공백을 만드는게 좋습니다.

  3. 브라켓({)이 시작하는 부분, 조건문과 반복문 내에서 등과 같이 띄워쓰기가 다소 일관성이 부족한 부분이 있었습니다.
    구글 스타일 가이드를 따르기로 했으니, 아래를 참고하시어 코드를 조금 더 깔끔하게 고쳐보면 좋을 것 같습니다.
    https://google.github.io/styleguide/cppguide.html


struct Node *MakeNode(int element)
{
struct Node *new_node_pointer = (struct Node *)malloc(sizeof(struct Node));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타입에서 드러나는 자료의 성질을 변수 이름으로 짓는 것을 피합니다.
new_node_pointer에서 pointer는 이미 포인터 변수이므로 불필요하고,
struct Node를 가리키고 있으므로 node도 불필요합니다.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 작명은 간결하면서도 표현하고자 하는 바를 온전히 명확하게 표현하는 작명입니다.


struct Node *FindNode(struct Node *linked_list, int key_element)
{
struct Node *checked_node = linked_list;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타입에서 드러나는 자료의 성질을 변수 이름으로 짓는 것을 피합니다.


void Insert(struct Node *linked_list, int prev_node_element, int element)
{
struct Node *prev_node = FindNode(linked_list, prev_node_element);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타입에서 드러나는 자료의 성질을 변수 이름으로 짓는 것을 피합니다.

struct Node *FindPrevNode(struct Node *linked_list, int key_element)
{
struct Node *checked_node = linked_list;
if(FindNode(linked_list, key_element) == NULL){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

두 번 탐색을 하는군요.
바로 직전의 방식을 채택하되 찾는 노드가 없을 때 null을 확인하는 방법이 있지 않을까요? :-)

}
}

void Delete(struct Node *list, int key_element)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

탐색 한 번 만에 삭제를 할 수 있는 방법이 있지 않을까요?

MakeNewList(&list);

for(int i=0; i<argc; i++){
printf("%s\n", argv[i]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

FILE *fp = fopen(argv[1],"r");

while(1){
if(fgets(buffer, sizeof(buffer), fp)==NULL) break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if(fgets(buffer, sizeof(buffer), fp) == NULL)
    break;

There should be spaces before and after operator
https://google.github.io/styleguide/cppguide.html#Conditionals


MakeNewList(&list);

for(int i=0; i<argc; i++){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for(int i = 0; i < argc; i++) {


while(1){
if(fgets(buffer, sizeof(buffer), fp)==NULL) break;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need whitespace

}

struct Node *FindNode(struct Node *linked_list, int key_element)
struct Node *FindNode(struct Node *list, int key_element)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

해당 커밋에서 feedback을 반영한 부분은 다음과 같다.

1. 구글 스타일 가이드에 따른 띄어쓰기 일관성 확보
2. 불필요한 공백, indent 제거
3. 일부 변수 이름에서 불필요한 정보 제거하여 다시 작명
해당 커밋에서 반영된 feedback은 다음과 같습니다.

1. FindPrevNode 함수에서 탐색을 한 번만 하도록 수정
2. Delete 함수에서 탐색을 한 번만 하도록 수정
3. ExecuteBufferLine 함수에서 if-else문 대신 switch-case문 활용하여 코드 구성
주어진 과제에서 Node를 찾을 때에는 student ID를 이용한다. 만약 student ID를 주어진 그대로
이용한다면 string data끼리 같고 다름을 비교해야 하는데, 이는 무척 번거로운 작업이다. 따라서
학번 string data를 각 학번마다 고유한 정수로 변환해주는 IdToInteger 함수를 정의하여 integer로
만든 후에 비교할 수 있도록 하였다.
Insert 함수의 경우 학생들의 학번 순서대로 list에 추가될 수 있도록 구현해야 하므로,
string data인 학번을  IdToInteger 함수를 사용하여 integer로 바꾼 뒤에 대소비교하였다.
main함수를 통해 주어진 과제의 input을 받고, ChooseOperations 함수를 통해 input에 따른
수행 동작을 결정하도록 한다.  ChooseOperations 함수에서 Command에 따라 변수를 다르게
선언한 이유는, insert 할 때 buffer를 통해 입력받은 데이터를 메모리에 따로 할당해 주어야 
buffer의 내용이 바뀌더라도 insert된 node들의 string data들이 변하지 않기 때문이다.
과제의 조건에 맞도록 main 함수에서 student_list의 최대 크기를 제한한다. Header node를 
고려하여 MAX_ENROLLMENT + 1개의 node가 들어갈 수 있도록 메모리를 할당한다.
@Deocksoo Deocksoo changed the title Linked list: basic 과제 리펙토링 완료 Linked list: basic, Linked list: application 과제 리펙토링 완료 Aug 10, 2018
Copy link
Contributor

@kwangilcho kwangilcho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 구현하셨어요!
코멘트 남겼습니다. :-)

}

int IsEmpty(struct Node *list) {
return (list->next == NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return list->next == NULL;

괄호가 없어도 괜찮을 것 같아요.

}

int IsLast(struct Node *node) {
return (node->next == NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

괄호가 없어도 괜찮을 것 같아요.

return (node->next == NULL);
}

int IdToInteger(char *student_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include <stdlib.h>
atoi(student_id)

str -> int


struct Node *FindNode(struct Node *list, char *student_id) {
struct Node *checked = list; //list의 header에서부터 찾기 시작
int integer_finding = IdToInteger(student_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그냥 finding은 어떤가요?

}
else {
struct Node *checked = list;
int integer_finding = IdToInteger(student_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finding?


void PrintList(struct Node *list) {
struct Node *printed = list->next;
while (printed!=NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space

case 'i': {
char *student_id_m = (char *)malloc(sizeof(char)*6);
char *student_name_m = (char *)malloc(sizeof(char)*30);
sscanf(buffer, "%c %s %[^\n]s", &command, student_id_m, student_name_m);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마지막 개행 문자 버리는 시도 좋네요.

}
}

void Insert(struct Node *list, char *student_id, char *student_name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수 로직을 조금 더 간결하게 정리해 볼 수 있을 것 같네요.


int main(int argc, const char *argv[]) {
struct Node *student_list;
struct Node *student_list = (struct Node *)malloc(sizeof(struct Node) * (MAX_ENROLLMENT + 1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants