Skip to content
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

[정수론] 9월 13일 #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[정수론] 9월 13일 #2

wants to merge 1 commit into from

Conversation

uommou
Copy link
Collaborator

@uommou uommou commented Sep 19, 2022

내용 & 질문

제출합니다!

<기존 제출>

1213, 2108, 2168, 14490

<추가 제출>

@0321minji
Copy link

2168 코드리뷰 완료
안녕하세요 채원님!!
제출해주신 코드 잘 봤습니다! (그런데 주석이 깨져서 너무 아쉽네요ㅜㅜ (https://yam-cha.tistory.com/120) 요 링크 참고해보시면 좋을 것 같아요:))
문제가 코드 자체는 짧지만 풀이 방법을 생각하기까지가 어려우셨을 텐데 잘 풀이해주셨네요👍👍
사소한 코멘트 남기니 참고 부탁드려요~!
수고하셨습니다🙌🙌

Copy link

@kwakrhkr59 kwakrhkr59 left a comment

Choose a reason for hiding this comment

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

p1. 1213 코드 리뷰 완료
안녕하세요 채원님~ 팰린드롬을 string과 아스키코드를 활용해 잘 구현해주셨더라구요👍
로직 자체는 너무나도 잘 구현을 해주셨는데 사용되지 않는 변수나 불필요한 for문 사용이 있어서 이부분에 대해서만 코멘트 남겨드렸습니다! 확인하시고 수정해주시고 리뷰어로 호출해주시면 감사하겠습니다😊
혹시 코멘트 읽으시면서 어려운 점이 계시면 언제든 질문주시면 빠르게 답변드리겠습니다:)

@@ -0,0 +1,66 @@
#include <iostream>;
#include <string>;

Choose a reason for hiding this comment

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

p3. #include <-->;처럼 include 뒤에는 ';'를 붙이지 않으셔도 괜찮습니다! (';'을 붙이시면 경고가 발생하네요)


for (int i = 0; i < str.length(); i++) {
int index = str[i] - 65;
arr[index]++;

Choose a reason for hiding this comment

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

p3. 'A'의 아스키코드가 65라는 것을 활용해서 인덱스를 지정해주신 것 너무 좋습니다👍👍 직접 'A'의 아스키코드를 찾아서 65라는 숫자로 코드를 작성해주셔도 좋지만, str[i]-'A'로 작성하셔도 전혀 문제가 발생하지 않습니다!

cout << "I'm Sorry Hansoo";
}

else {

Choose a reason for hiding this comment

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

p3. if로 홀수가 1개 이상일 때와 아닐 때의 코드를 나눠서 작성해주셨는데요! odd가 1보다 클 때를 if문으로 감싸서 cout 해주신 뒤 return을 해주시면 else를 사용하지 않고도 코드가 문제 없이 돌아갈 것 같네요:)
return을 잘 활용해주시면 인덴테이션도 줄여서 훨씬 더 깔끔하게 코드를 구현하실 수 있으실 거예요😁

for (int i = 0; i < 26; i++) {
if (arr[i] % 2 != 0) {
odd_index == i;
flag = true;

Choose a reason for hiding this comment

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

p2. flag가 이후에 사용되고 있지 않습니다! 백준에서 코드를 돌려보시고 작성하신 코드를 확인해보시면 코드를 실행하면서 발생하는 경고를 확인해보실 수 있을 거예요:) 이렇게 이후에 활용되지 않는 변수는 삭제해주시면 메모리 관리도 할 수 있으니 유의해주시면 좋을 것 같습니다!

sol += i + 'A';
}
}
}

Choose a reason for hiding this comment

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

p2. 저희 튜터링에서는 클린코드 작성을 위해 입출력을 제외한 로직 부분은 함수화하는 것을 장려하고 있습니다! 팰린드롬을 생성하는 부분을 함수로 작성해보시면 좋을 것 같네요:)



int odd = 0;
int odd_index;

Choose a reason for hiding this comment

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

p2. odd_index가 사용되고 있지 않습니다! 밑에서 odd의 개수를 세서 홀수 개수가 1개보다 많을 때를 if문으로 분류해주고 계신데 홀수 개수를 세는 부분에서 odd를 홀수 개수가 아니라 홀수 개인 알파벳으로 설정해주시면 추후에 if문 없이 한번에 홀수 개인 알파벳이 1개보다 많을 때를 구별해볼 수 있으실 것 같습니다😊


for (int i = 0; i < 26; i++)
if (arr[i] % 2)
sol += i + 'A';

Choose a reason for hiding this comment

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

p1. 위에서 odd로 가운데에 올 문자를 지정할 수 있는 방법에 대해서 남겨드렸습니다! 이 방법을 활용하면 해주셨으니 굳이 같은 의미를 가지는 for문을 2번씩 돌면서 가운데에 올 문자를 찾지 않아도 될 것 같습니다😊

Copy link

@grdnr13 grdnr13 left a comment

Choose a reason for hiding this comment

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

p3. 2108번 코드리뷰 완료
안녕하세요, 채원님! 코드 깔끔하게 작성해주셨네요. 주석이 깨진 것 같아서 한번 확인해주시면 감사하겠습니다
사소한 코멘트 남겼으니 참고해주세요! 수고하셨습니다🥰


}

// ��� - �Ҽ�� ���� ù���ڸ� �ݿø�
Copy link

Choose a reason for hiding this comment

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

제출하시면서 주석이 깨진 것 같아요! 확인부탁드립니다

cin >> n;
sum += n;
v.push_back(n);
arr[n+4000]++;
Copy link

Choose a reason for hiding this comment

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

p3. 4000이 문제에서 주어지는 숫자 범위이니, 상수로 선언하고 사용하시면 더 편할거에요!

int count;
cin >> count;

int sum = 0;
Copy link

Choose a reason for hiding this comment

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

p3. sum을 double로 선언하면, 뒤에서 형변환을 거칠 필요가 없어져요!
(자료형을 섞어서 연산을 하면 컴파일러에서 암시적 형 변환을 해줍니다)

Comment on lines +36 to +37
int med = v.at(v.size() / 2);
cout << med << "\n";
Copy link

Choose a reason for hiding this comment

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

p3. 변수가 한 번 쓰이고 마니, 바로 리턴에 넣어서 한 줄로 합치면 어떨까요?

Comment on lines +40 to +56
int stop = 0;
int com;
int common_value = 0;
for (int i = 0; i < 8001; i++) {
if (arr[i] > common_value) {
common_value = arr[i];
}
}
for (int i = 0; i < 8001; i++) {
if (arr[i] == common_value) {
com = i-4000;
stop++;
}
if (stop == 2) {
break;
}
}
Copy link

Choose a reason for hiding this comment

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

p3. 너무 잘 구현해주셨어요!
재사용성을 위해, 최빈값 구하는 부분은 함수화하는게 어떨까요?

Comment on lines +60 to +61
int rng = v.back() - v.front();
cout << rng;
Copy link

Choose a reason for hiding this comment

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

p3. rng가 한 번 쓰이고 마니, 바로 리턴에 넣어서 한 줄로 합쳐도 될 것 같아요!

Copy link

@junghk0115 junghk0115 left a comment

Choose a reason for hiding this comment

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

14490 코드리뷰 완료
안녕하세요 채원님!:)
코드 너무 완벽해서 따로 드릴 코멘트는 없습니다~ 👍 👍
과제 하시느라 수고 많으셨습니다

Comment on lines +21 to +23
int gcd = gcdRecursion(x, y);

cout << x + y - gcd;

Choose a reason for hiding this comment

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

p3.
gcd 변수는 함수의 값만 return 받고 출력하는 기능만 하기에 변수 선언을 해주지 않고 바로 출력문에 병합하여 작성할 수 있을 것 같아 보이네요~~!

@uommou uommou changed the title #3 [정수론] 9월 13일 [정수론] 9월 13일 Sep 22, 2022
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.

5 participants