Skip to content

Conversation

@Yoonjiho17
Copy link

4주차 과제 제출합니다.

Copy link

@Gwak-Seungju Gwak-Seungju left a comment

Choose a reason for hiding this comment

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

api 연결 잘 하셨습니다! 고생하셨어요~~! 😀

Comment on lines +1 to +6
document.getElementById("searchButton").addEventListener("click", async function() {
const countryName = document.getElementById("countryInput").value
if (!countryName) {
alert("나라 이름을 입력하세요!");
return;
}

Choose a reason for hiding this comment

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

함수 안에 내용이 많아서 이 함수가 어떤 기능을 하는 함수인지 잘 모르겠어요..! 따라서 익명 함수보다는 함수 이름을 지정해서 어떤 함수인지 함수명을 통해 알 수 있게 하면 좋을 것 같아요! 🙂

Copy link
Author

Choose a reason for hiding this comment

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

넵 익명 함수를 활용하기 보다는 함수 이름을 지정해서 가독성을 키울 수 있는 코드를 짜도록 하겠습니다. 감사합니다!

Comment on lines +10 to +13
if (response.status === 404) {
document.getElementById("result").innerText = "해당 나라를 찾을 수 없습니다.";
return;
}

Choose a reason for hiding this comment

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

const result = document.getElementById("result");

result.innerText = " ";

와 같이 가독성있게 작성하는 것도 좋을 것 같아요! 아래에도 result 객체가 또 쓰일 수도 있고 하니까 !

Copy link
Author

Choose a reason for hiding this comment

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

if문 밖에 const result = document.getElementById("result");를 정의하였더니 재사용이 가능해져서 코드도 줄어들고 가독성이 좋아졌습니다. 조언 감사합니다!

Comment on lines +15 to +16
border-radius: 5px;
}

Choose a reason for hiding this comment

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

border-radius를 지정한 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

모서리를 둥글게 만드는 게 더 가시성있다고 생각하여 지정하였습니다. 디자인적 요소가 아닌 다른 의도한 바는 없습니다!

Comment on lines +18 to +21
#result {
margin-top: 20px;
margin-bottom: 20px;
}

Choose a reason for hiding this comment

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

만약 위 아래만 margin을 주고 싶다면, margin-topmargin-bottom을 따로 지정하지 않고
margin: 20px 0; 으로 하면 왼쪽은 상하, 오른쪽은 좌우 px을 지정할 수 있습니다!

Copy link
Author

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