프론트엔드 코드리뷰 #1

JunilHwang · 2021-08-15T08:10:01Z

No description provided.

JunilHwang

전체적인 리뷰를 종합해보자면 다음과 같습니다.

고차 컴포넌트 활용하기
state를 그대로 사용하기보단 구조분해할당을 이용하여 명시적으로 변수를 선언해보기
useCallback을 이용하여 렌더링이 될 때 마다 동일한 함수가 선언되는 현상 방지하기
- 즉, 메모이제이션을 적극적으로 사용해보세요!
export default 변수명 형태로 export 해보기
- 이렇게 해야 에디터나 IDE에서 auto import를 하거나 rename을 할 때 수월하답니다!
불필요한 catch문 제거하기
- catch에서 단순하게 throw error를 하는 것은 의미가 없답니다!
- 오류가 발생했을 때 후처리가 필요한 코드를 넣거나 아예 제거해도 무방합니다.
적절하게 삼항연산자를 사용하는 것은 좋지만, 가독성을 항상 생각해주세요! 일부 코드는 삼항연사자 때문에 가독성이 무척 좋지 않은 상태입니다.
사용중인 패키지가 deprecated 되지 않는지 확인해보기
- momentjs나 crypto 같은 경우 공식적으로 지원이 종료된 패키지입니다.
- 대체 패키지를 사용해보세요!

이 외에 상세한 내용은 리뷰 내용을 확인해보시면 좋을 것 같습니다 😁
수고하셨습니다!

JunilHwang · 2021-08-15T08:11:25Z

review_practice/api-server/app.js

+    console.error('Path:', err.path);
+    console.error('Message:', err.message);
+    console.error('Code:', err.extensions.code);
+    console.error('Original Error', err.originalError);


destructuring 해서 사용해보면 어떨까요?

JunilHwang · 2021-08-15T08:13:42Z

review_practice/api-server/config/database.js

+
+module.exports = {
+    development: {
+        username: DB_DEV_USER,
+        password: DB_DEV_PASSWORD,
+        database: DB_DEV_DATABASE,
+        host: DB_DEV_HOST,
+        dialect: 'mysql',
+    },
+    test: {
+        username: DB_DEV_USER,
+        password: DB_DEV_PASSWORD,
+        database: DB_DEV_DATABASE,
+        host: DB_DEV_HOST,
+        dialect: 'mysql',
+    },


Suggested change

module.exports = {

development: {

username: DB_DEV_USER,

password: DB_DEV_PASSWORD,

database: DB_DEV_DATABASE,

host: DB_DEV_HOST,

dialect: 'mysql',

},

test: {

username: DB_DEV_USER,

password: DB_DEV_PASSWORD,

database: DB_DEV_DATABASE,

host: DB_DEV_HOST,

dialect: 'mysql',

},

const DEV_ENV = {

username: DB_DEV_USER,

password: DB_DEV_PASSWORD,

database: DB_DEV_DATABASE,

host: DB_DEV_HOST,

dialect: 'mysql',

};

module.exports = {

development: { ...DEV_ENV },

test: { ...DEV_ENV },

이렇게 사용하면 확장하고 유지하기가 편리할 것 같아요!

JunilHwang · 2021-08-15T08:17:03Z

review_practice/api-server/database/models/index.js

+
+fs.readdirSync(__dirname)
+    .filter(file => {
+        return file.indexOf('.') !== 0 && file !== basename && file.slice(-3) === '.js';


Suggested change

return file.indexOf('.') !== 0 && file !== basename && file.slice(-3) === '.js';

return !file.startWith('.') && file !== basename && file.endWith('.js');

이렇게 startWith와 endWith라는 메소드를 사용해도 된답니다!

JunilHwang · 2021-08-15T08:19:21Z

review_practice/api-server/database/models/index.js

+Object.keys(db).forEach(modelName => {
+    if (db[modelName].associate) {
+        db[modelName].associate(db);
+    }
+});


일단 Object.keys 대신에 values를 사용해보면 어떨까요?

Suggested change

Object.keys(db).forEach(modelName => {

if (db[modelName].associate) {

db[modelName].associate(db);

}

});

Object.values(db)

.filter(model => model.associate)

.forEach(model => model.associate(db));

그리고 다음과 같이 optional channing을 이용하는 방법도 있답니다!

Suggested change

Object.keys(db).forEach(modelName => {

if (db[modelName].associate) {

db[modelName].associate(db);

}

});

Object.values(db).forEach(model => model.associate?.(db));

JunilHwang · 2021-08-15T08:20:40Z

review_practice/api-server/database/seeders/20191017020042-init-room-type.js

+                    name: '개인실',
+                },
+                {
+                    name: '호텔 객실',


오타가 있네요!

JunilHwang · 2021-08-15T09:21:06Z

review_practice/react-app/src/containers/SearchBar/SearchButton.js

+import Modal from 'components/Modal';
+import { isMobile } from 'Constants';
+
+export default ({ name, modalBody, isClicked, setIsClicked, onClick }) => {


앞선 리뷰를 참고해주세요!

JunilHwang · 2021-08-15T09:21:23Z

review_practice/react-app/src/containers/SearchBar/SearchButton.js

+            >
+                {name}
+            </button>
+            {!isMobile() && isClicked && <Modal body={modalBody} />}


앞선 리뷰(고차 컴포넌트)를 참고해주세요!

JunilHwang · 2021-08-15T09:22:03Z

review_practice/react-app/src/containers/SearchBar/index.js

+    const buttonModalBodyList = [
+        (buttonState, setButtonState) => <DateFilter buttonState={buttonState} setButtonState={setButtonState} />,
+        (buttonState, setButtonState) => <PersonnelFilter buttonState={buttonState} setButtonState={setButtonState} />,
+        (buttonState, setButtonState) => <PriceFilter buttonState={buttonState} setButtonState={setButtonState} />,
+    ];


이건 함수 외부에서 선언해줘도 될 것 같아요!

JunilHwang · 2021-08-15T09:22:28Z

review_practice/react-app/src/containers/SearchBar/index.js

+    const searchStates = buttonStateList.map((state, index) => {
+        return {
+            name: state[0].buttonName,
+            modalBody: buttonModalBodyList[index](state[0], state[1]),
+            isClicked: state[0].isClicked,
+            setIsClicked: isClicked => {
+                state[1]({ ...state[0], isClicked });
+            },
+        };
+    });


Suggested change

const searchStates = buttonStateList.map((state, index) => {

return {

name: state[0].buttonName,

modalBody: buttonModalBodyList[index](state[0], state[1]),

isClicked: state[0].isClicked,

setIsClicked: isClicked => {

state[1]({ ...state[0], isClicked });

},

};

});

const searchStates = buttonStateList.map((state, index) => ({

name: state[0].buttonName,

modalBody: buttonModalBodyList[index](state[0], state[1]),

isClicked: state[0].isClicked,

setIsClicked: isClicked => {

state[1]({ ...state[0], isClicked });

},

}));

이렇게 표현할 수 있답니다!

JunilHwang · 2021-08-15T09:22:54Z

review_practice/react-app/src/containers/SearchBar/index.js

+        (buttonState, setButtonState) => <PriceFilter buttonState={buttonState} setButtonState={setButtonState} />,
+    ];
+
+    const searchStates = buttonStateList.map((state, index) => {


state를 분해하여 사용해보면 어떨가요?

connectfoundation added 20 commits August 12, 2021 10:44

Update README.md

4f52d28

Update README.md

adbf62c

Add files via upload

172d937

Update README.md

a005751

Update README.md

5795ec0

Update README.md

fa69842

Update README.md

7c49622

Update README.md

d639dbe

Update README.md

5c292a3

Update README.md

a6e7674

Update README.md

e85ccb0

Update README.md

27d7cb0

Update README.md

b7f7d55

Update README.md

289922f

Update README.md

afdf95f

Update README.md

3228e23

Update README.md

3306cad

Update README.md

346cb9b

Update README.md

0627e2f

Update README.md

0905019

JunilHwang changed the title ~~[Web Frontend]~~ [Frontend] 코드리뷰 Aug 15, 2021

JunilHwang changed the title ~~[Frontend] 코드리뷰~~ 프론트엔드 코드리뷰 Aug 15, 2021

JunilHwang commented Aug 15, 2021

View reviewed changes

JunilHwang mentioned this pull request Aug 18, 2021

2021 08 19 DKU-STUDY/TodayReview#495

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

프론트엔드 코드리뷰 #1

프론트엔드 코드리뷰 #1

JunilHwang commented Aug 15, 2021

JunilHwang left a comment •

edited

Loading

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

JunilHwang Aug 15, 2021

	return file.indexOf('.') !== 0 && file !== basename && file.slice(-3) === '.js';
	return !file.startWith('.') && file !== basename && file.endWith('.js');

프론트엔드 코드리뷰 #1

Are you sure you want to change the base?

프론트엔드 코드리뷰 #1

Conversation

JunilHwang commented Aug 15, 2021

JunilHwang left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JunilHwang left a comment •

edited

Loading