Skip to content

Conversation

shongs27
Copy link

@shongs27 shongs27 commented Dec 3, 2021

pull request가 삭제되서 다시 올립니다.

전역변수를 사용하는 방법으로 구현을 했고 사용하지 않는 방법으로 수정해나가겠습니다.

코딩 처음 배울때 계산기 만들기를 잠깐 해봤었는데, 오랜만에 만들려니깐 못하겠더라구요. 해결책이 보이지 않아도 어떻게든 혼자만의 힘으로 구현해보려고 노력하는게 좋을까요, 아니면 인터넷에 계산기 프로젝트를 참고해서 수정해보는 식의 학습도 괜찮을 걸까요?

Copy link
Collaborator

@moonkii moonkii left a comment

Choose a reason for hiding this comment

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

image

전역변수를 사용하는 방법으로 구현을 했고 사용하지 않는 방법으로 수정해나가겠습니다.

잘 하셨어요 👍
과제를 진행하는 프로세스는 우선 '작동'하는 소프트웨어를 만들고나서 리팩터링해 나가는 점진적인 형태라고 생각하시면 좋아요.

코딩 처음 배울때 계산기 만들기를 잠깐 해봤었는데, 오랜만에 만들려니깐 못하겠더라구요. 해결책이 보이지 않아도 어떻게든 혼자만의 힘으로 구현해보려고 노력하는게 좋을까요, 아니면 인터넷에 계산기 프로젝트를 참고해서 수정해보는 식의 학습도 괜찮을 걸까요?

과제가 미완성일지라도 먼저 PR을 보내라는 이유가 여기에 있어요!
과제를 해결하기 어렵거나 방향성을 찾기 힘들 때는 빠르게 PR을 보내거나 슬랙 DM을 보내 짧은 주기로 피드백을 주고 받으면 좋아요.
계산기 자체를 만드는 것도 중요하긴 하지만 먼저 과제의 의도를 파악하려고 고민해보시면 좋아요.
그러면 인터넷에 있는 계산기 프로젝트를 참고하는게 아무 의미가 없다는걸 알 수 있으실거에요.
앞으로 점점 주차가 지날수록 이때까지 배우지 못한 그리고 인터넷에서는 찾을 수 없는 내용들을 배우시게 될거에요. 그렇기 때문에 어차피 혼자 힘으로 해결할 수 밖에 없기도 해요 ;)
(요약 하자면 대략 스스로의 힘으로 해야한다는 그런 말이에요 😇 )

src/index.js Outdated
Comment on lines 6 to 13
const state = {
firstValue: '',
secondValue: '',
firstDone: false,
secondDone: false,
curOper: '',
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

과제 1과 마찬가지로 객체를 이용하기 보단 함수의 매개변수를 이용해 값을 전달하는 방식으로 문제를 해결해보세요 ;)

src/index.js Outdated
Comment on lines 36 to 49
switch (state.curOper) {
case '+':
return intValueA + intValueB;
case '-':
return intValueA - intValueB;
case '/':
return intValueA / intValueB;
case '*':
return intValueA * intValueB;
case '':
return intValueA;
default:
}
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

switch 문은 대표적으로 코드 가독성을 떨어뜨리는 문법 중 하나에요.
객체를 이용해서 표현해볼 수 있을까요?

혹시 클린코드 책을 안 읽어 보셨다면 추천드려요!

Copy link
Author

@shongs27 shongs27 Dec 4, 2021

Choose a reason for hiding this comment

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

책 추천 감사합니다! 꼭 읽어보도록 할게요. 책 추천으로 많이 본 책인데 굉장히 좋은 책인가봐요 :)

Switch는 되도록 지양해야 하는거군요.. 배웠습니다.

const fundamentalOperation = {
      '+': intValueA + intValueB,
      '-': intValueA - intValueB,
      '/': intValueA / intValueB,
      '*': intValueA * intValueB,
    };
    return fundamentalOperation[state.curOper];

객체를 사용하니깐 간결하고 직관적이네요.

src/index.js Outdated
Comment on lines 62 to 70
if (!state.firstDone) {
state.firstValue += n;
render(state.firstValue);
} else {
state.secondValue += n;
state.secondDone = true;
render(state.secondValue);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

if ~ else 문을 사용하지않고 코드를 표현할 수 있는 방법이 있을까요?

힌트를 드리자면 guard clauses 를 찾아보시면 도움이 될거에요 ;)

Copy link
Author

@shongs27 shongs27 Dec 4, 2021

Choose a reason for hiding this comment

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

function 아무개(){
  if (!firstDone) return render({ firstValue, firstDone: true, currentOperation: operation });
  if (firstDone && secondDone) {
      return calculate(operation);
    }
  return 0; // 의미없는 코드인데 왜 eslint는 강제하는걸까?
}

알려주신대로 nested되는 항목을 빼는 guard clauses를 사용했습니다.
다양한패턴에 익숙하게 사용하도록 연습을 많이 해야겠다고 생각했어요.

조금 다른 질문인데요, 여기서 Eslint가 반드시 함수의 return 값을 요구하더라구요 consistent-return
return 0; 으로 임시방편으로 어거지 return값을 설정했는데요. 그냥 return; 도 되지 않고 eslint에서 반드시 값을 요구하는 이유가 있나요?

src/index.js Outdated
Comment on lines 93 to 100
<button type="button" onClick={() => onClickNum(i)}>{i}</button>
))}
</div>
<br />
<div>
{['+', '-', '*', '/', '='].map((s) => (
<button type="button" onClick={() => onClickOper(s)}>{s}</button>
))}
Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 네이밍을 할 때 축약형을 지양해요.
예를들어 사소하지만 number -> 숫자 / num -> number -> 숫자 처럼 코드를 읽는 사람으로 하여금 불필요한 인지자원을 소모하게 해서 직관적인 이해를 하는데 방해가 되는 것 같아요.
또한 서로 미리 충분히 숙지하지 않은 경우 코드를 읽는 사람은 의도를 파악할 수도 없어 코드 가독성에 굉장히 나쁜 영향을 미칠거에요.

src/index.js Outdated
Comment on lines 104 to 105
document.getElementById('app').textContent = '';
document.getElementById('app').appendChild(element);
Copy link
Collaborator

Choose a reason for hiding this comment

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

중복되는 코드를 어떻게 정리해줄 수 있을까요?

Copy link
Author

@shongs27 shongs27 Dec 4, 2021

Choose a reason for hiding this comment

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

첫번쨰 과제와 같이 동일한 방법으로 바꾸었습니다

 const app = document.getElementById('app');
  app.textContent = '';
  app.appendChild(element);

src/index.js Outdated
}
}

function onClickOper(s) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

s 는 무엇을 의미하나요?
코드를 처음 보는 사람은 이 이름을 어떻게 이해할 수 있을까요?

Copy link
Author

@shongs27 shongs27 Dec 4, 2021

Choose a reason for hiding this comment

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

음.. 처음에 string이라는 의미로 s를 설정을 했는데요. 매개변수에도 명확한 의미가 필요하다는 것을 알고 onClickOperation(arithmetic)로 바꾸어 주었습니다. 그런데 제가 네이밍한 인수가 읽는 상대로 하여금 의미가 확 닿지는 않은것 같아요.. 사칙연산(+,-,*,/)를 표현하고 싶어서 arithmetic 이라는 변수로 지었습니다

@shongs27
Copy link
Author

shongs27 commented Dec 4, 2021

피드백 참고해서 구현했습니다. 정말 도움이 많이 되었습니다. 느낀것도 많구요.

그런데 전역변수를 사용하는 것보다 매개변수를 통해서 상태값을 전달해주는 과정이 조금 부자연스럽다는 생각이 들었습니다. 왜냐하면 인수전달로 상태를 구현하다보니 생각외로 화면을 다시 그려주는 render 함수를 불러주는 경우가 많이 생기더라구요. 이게 리액트가 상태값이 변경될때마다 다시 그려주는것과 유사하구나하면서도 자원낭비가 아닌가 하는 생각이 들어요. 인수 전달로 하나하나 구현하기가 귀찮고 렌더링을 자주 해주는 듯 싶지만 전역변수를 사용하는 것보다는 올바른건가요?

늦은 시간에도 리뷰 감사합니다! 피드백들 정리한 뒤에 주간회고도 올리겠습니다 :)

@shongs27 shongs27 mentioned this pull request Dec 5, 2021
@moonkii
Copy link
Collaborator

moonkii commented Dec 5, 2021

그런데 전역변수를 사용하는 것보다 매개변수를 통해서 상태값을 전달해주는 과정이 조금 부자연스럽다는 생각이 들었습니다. 왜냐하면 인수전달로 상태를 구현하다보니 생각외로 화면을 다시 그려주는 render 함수를 불러주는 경우가 많이 생기더라구요. 이게 리액트가 상태값이 변경될때마다 다시 그려주는것과 유사하구나하면서도 자원낭비가 아닌가 하는 생각이 들어요. 인수 전달로 하나하나 구현하기가 귀찮고 렌더링을 자주 해주는 듯 싶지만 전역변수를 사용하는 것보다는 올바른건가요?

정답이 있는건 아니에요.
trade off 라고 생각하시면 좋아요. 말씀하신대로 render 함수를 계속 호출해서 전체를 다시 그려준다면 자원낭비가 될 수 있겠죠.
리액트에서는 그래서 변경된 부분만 랜더링해주는 형태로 자원낭비를 막는 방법을 쓰기도 해요.
하지만 함수형 프로그래밍으로 immutable 하게 코드를 짤 수 있죠.
자원의 효율성이냐 코드의 안전성이냐 뿐만 아니라 각각의 장단점에 따라 vs 가 될 수 있겠죠?
저는 개인적으로 자원의 효율성 때문에 발생하는 문제 보다는 프로그래머의 실수나 인지 부조화에 따른 문제에 더 가중치를 두고 코드를 작성하는 편이에요. 결국 어디에 더 가중치를 두느냐에 따라 코드의 형태가 달라진다고 보시면 될 것 같아요.

Copy link
Collaborator

@moonkii moonkii left a comment

Choose a reason for hiding this comment

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

풀이 영상을 통해 어떻게 코드를 작성하고 문제를 해결해 나가는지 과정을 잘 살펴보시면 좋아요.
지금 작성하신 코드들과 어떤 차이점이 있는지도 비교해보시면 도움이 많이 되실거에요.
주간회고를 공유해주셨기 때문에 merge 하도록하겠습니다!

@moonkii moonkii merged commit 99251e2 into CodeSoom:shongs27 Dec 5, 2021
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