Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[tracy] react-calculator 구현 완료입니다.#5

Open
PParkJy wants to merge 2 commits into
codewear-study:tracy from
PParkJy:tracy
Open

[tracy] react-calculator 구현 완료입니다. #5
PParkJy wants to merge 2 commits into
codewear-study:tracy from
PParkJy:tracy

Conversation

@PParkJy

@PParkJy PParkJy commented Jan 16, 2020

Copy link
Copy Markdown

pull request 에러가 계속 나길래 해결 방법을 찾다가 저장소를 지우고 새로 만들어버렸습니다.
그래서 이전 commit 내용들이 다 날아가버렸습니다....껄껄....
저처럼 멍청하게 하지마세요 흑흑

@PParkJy PParkJy changed the title (削除) [tracy] "react-calculator" 구현 완료입니다. (削除ここまで) (追記) [tracy] react-calculator 구현 완료입니다. (追記ここまで) Jan 16, 2020

Copy link
Copy Markdown

고생하셨습니다!!

Copy link
Copy Markdown

으앗.. gitignore파일 지정해 주세요:sob: 모듈 파일때문에 코드 쓰신게 하나도 안떠요

PParkJy commented Jan 16, 2020

Copy link
Copy Markdown
Author

gitignore 추가했습니다!

@Tetramad Tetramad left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

굿:+1:

Comment on lines +3 to +4
const preOperand = parseFloat(state.displayValue || "0")
const backOperand = parseFloat(state.nextValue || (operator === "/" || operator === '*' ? "1": "0"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

preOperand, backOperand라는 변수명보다 lhs, rhs 또는 lhsOperand, rhsOperand를 사용하는 것을 추천합니다. left-hand side, right-hand side의 약자입니다.
state.displayValue || "0"로 null 처리하는 것은 좋아요! 👍

Comment on lines +2 to +8
if (next) {
if (next.includes(".")) {
return {};
}
return { nextValue: next + "." };
}
return { nextValue: "0." };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

들여쓰기 간격 맞춰주세요!

Copy link
Copy Markdown
Member

고생하셨습니다!

@Parkhyunseo Parkhyunseo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

고생하셨습니다!! 전체적으로 코드가 이전에 비해 많이 깔끔해졌습니다. 그러나 아직 null과 빈 객체를 많이 사용하고 명확하지 않은 코드들이 보이네요. 그리고 React 컴포넌트를 조금 더 활용했으면 좋겠습니다!

@@ -0,0 +1,11 @@
function decimalPoint(next){

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

함수명은 Naming Convention에 의거하여 동사를 사용해야 합니다.

@@ -0,0 +1,11 @@
function decimalPoint(next){
if (next) {

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if(next)가 의미하는게 명확하지 않습니다.
next === NaN, next === undefined 이런식으로 정확히 명시해주세요.

const operator = state.operatorValue
const preOperand = parseFloat(state.displayValue || "0")
const backOperand = parseFloat(state.nextValue || (operator === "/" || operator === '*' ? "1": "0"))
if (operator === "+") {

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

여유가 된다면 string연산자와 대응하는 함수를 객체로 묶어서 개발해 보는걸 추천합니다.

return (preOperand/backOperand).toString();
}
}
console.log('error');

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이번 요구사항에 console 사용은 없습니다! 지워주세요.

render(){
return(
<Fragment>
<button name="C" onClick={event => this.props.click(event.target.name)}>C</button>

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

버튼도 모두 React Component로 만들어보는 걸 추천합니다.

import DisplayLayout from "./displayLayout"
import ButtonLayout from "./buttonLayout"

class reactCalculator extends Component{

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

클래스명은 대문자로 시작합니다

@@ -0,0 +1,12 @@
import React, {Component } from 'react';

class displayLayout extends Component{

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

클래스명은 대문자로 시작합니다.

return {};
}

if (operator) {

@Parkhyunseo Parkhyunseo Jan 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indent가 맞지 않네요

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

Reviewers

@Parkhyunseo Parkhyunseo Parkhyunseo left review comments

@HodaeSsi HodaeSsi Awaiting requested review from HodaeSsi

@leaf-upper leaf-upper Awaiting requested review from leaf-upper

+1 more reviewer

@Tetramad Tetramad Tetramad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /