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

[FE] 우리 - 크리스마스 프로모션 미션 코드리뷰용! #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
node_modules
package-lock.json
.DS_Store
package-lock.json
.gitignore
336 changes: 81 additions & 255 deletions README.md

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions __tests__/DateTest.js
Copy link

Choose a reason for hiding this comment

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

특수문자가 포함된 케이스라던지, 소숫점이 들어간 숫자 등 혹시 모르니 여러 케이스를 테스트 해보는건 어떨까 싶어요!
저도 여러 케이스로 테스트코드를 작성하다가 버그를 수정할 수 있던 적이 몇번 있더라구요

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import Date from '../src/Model/Date';
describe('날짜 입력값 테스트', () => {
const INVALID_DATE_MESSAGE = '[ERROR]';
test('0 이하 숫자 입력', () => {
const INVALID_DATE = '0';
expect(() => new Date(INVALID_DATE)).toThrow(INVALID_DATE_MESSAGE);
});

test('32 이상 숫자 입력', () => {
const INVALID_DATE = '1000';
expect(() => new Date(INVALID_DATE)).toThrow(INVALID_DATE_MESSAGE);
});

test('숫자가 아닌 문자 입력', () => {
const INVALID_DATE = 'abcd';
expect(() => new Date(INVALID_DATE)).toThrow(INVALID_DATE_MESSAGE);
});
});
25 changes: 25 additions & 0 deletions __tests__/MenuTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import Menu from '../src/Model/Menu';

describe('메뉴 이름과 수량 입력값 테스트', () => {
const INVALID_MENU_MESSAGE = '[ERROR] 유효하지 않은 주문입니다. 다시 입력해 주세요.';
test('형식에 맞지 않은 입력값', () => {
const INVALID_MENU = '크리스마스파스타4';
expect(() => new Menu(INVALID_MENU)).toThrow(INVALID_MENU_MESSAGE);
});
test('메뉴 개수 0개 입력', () => {
const INVALID_MENU = '크리스마스파스타-0,와인-0';
expect(() => new Menu(INVALID_MENU)).toThrow(INVALID_MENU_MESSAGE);
});
test('음료 1개 입력', () => {
const INVALID_MENU = '와인-1';
expect(() => new Menu(INVALID_MENU)).toThrow(INVALID_MENU_MESSAGE);
});
test('매뉴 총 개수 20개 초과', () => {
const INVALID_MENU = '크리스마스파스타-10,와인-20';
expect(() => new Menu(INVALID_MENU)).toThrow(INVALID_MENU_MESSAGE);
});
test('중복 메뉴 입력', () => {
const INVALID_MENU = '크리스마스파스타-1,크리스마스파스타-2,와인-3';
expect(() => new Menu(INVALID_MENU)).toThrow(INVALID_MENU_MESSAGE);
});
});
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

81 changes: 80 additions & 1 deletion src/App.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,84 @@
import InputView from './View/InputView.js';
import OutputView from './View/OutputView.js';
import Date from './Model/Date.js';
import Menu from './Model/Menu.js';
import Event from './Model/Event.js';
import { AMOUNT, ASK, DIVIDER_HYPHEN, GREETING, HEADER, MENU, SEE_MORE_ABOUT_EVENTS } from './constants.js';
Copy link

Choose a reason for hiding this comment

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

/Model/index.js

import {default as Date} from './Model/Date.js';
import {default as Menu} from './Model/Menu.js';
import {default as Event} from './Model/Event.js';

src/App.js

import {Date, Menu, Event} from './Model/index.js'

Model 폴더 아래에 index.js 파일을 위와 같이 작성해서 만들면 import가 조금 더 간단하게 될 수 있더라구요!
저도 다른 분들이 알려줘서 이렇게 적용해보고 있습니다 ㅎㅎ

import menusData from './menus.js';

class App {
async run() {}
constructor() {
this.inputView = new InputView();
this.outputView = new OutputView();
}
async run() {
this.outputView.print(GREETING);
let dateInput;
try {
dateInput = await this.#getDateInput();
} catch (error) {
this.outputView.print(error.message);
dateInput = await this.#getDateInput();
}
const date = new Date(dateInput);
let menuInput;
try {
menuInput = await this.#getMenuInput();
} catch (error) {
this.outputView.print(error.message);
menuInput = await this.#getMenuInput();
}

Choose a reason for hiding this comment

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

해당 부분은 같은 로직이 date, menu 에 따라 반복되고 있어서, 하나의 메서드로 구현하면 좋을 것 같습니다.

Copy link

Choose a reason for hiding this comment

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

제가 혹시 코드에 대한 이해가 부족해서 드리는 질문일 수도 있는데,
다음과 같은 상황에서도 계속 입력을 받나요??

1. 잘못된 date 값 입력 -> 에러 출력
2. 한번 더 잘못된 date 값 입력

뭔가 코드상에선 1번만 예외처리를 하는 것 같이 보여서요!
직접 실행해볼 수 없으니 질문드릴 수 밖에 없군요 ㅜㅜ

Copy link
Author

Choose a reason for hiding this comment

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

제가 혹시 코드에 대한 이해가 부족해서 드리는 질문일 수도 있는데, 다음과 같은 상황에서도 계속 입력을 받나요??

1. 잘못된 date 값 입력 -> 에러 출력
2. 한번 더 잘못된 date 값 입력

뭔가 코드상에선 1번만 예외처리를 하는 것 같이 보여서요! 직접 실행해볼 수 없으니 질문드릴 수 밖에 없군요 ㅜㅜ

음 그렇네요..! 생각해보니 이런 형태면 두번째 입력 때는 에러로 아예 프로그램이 끝날 것 같아요..! 저도 한번 다시 테스트를 해봐야 하는 부분이네요. try&catch를 getInput 함수 안에 넣었으면 괜찮았을까요??

const menu = new Menu(menuInput);
this.outputView.print(SEE_MORE_ABOUT_EVENTS(date.getDate()));
this.outputView.print(HEADER.MENU);
const foodArr = menu.getFoodArr();
foodArr.forEach((food) => {
const name = food.getName();
const amount = food.getAmount();
this.outputView.printMenu(name, amount, DIVIDER_HYPHEN);
});
this.outputView.print(HEADER.TOTAL_PRICE_BEFORE_DISCOUNT);
const totalPricePreDiscount = menu.getTotalPrice();
this.outputView.printMoney(totalPricePreDiscount);
const event = new Event(date, menu);
this.outputView.print(HEADER.GIVEAWAY_MENU);
if (event.validateGiveAwayEvent(totalPricePreDiscount)) {
const [giveAwayFoodName, giveAwayFoodAmount] = event.getGiveAwayInfo();
this.outputView.printMenu(giveAwayFoodName, giveAwayFoodAmount);
} else {
this.outputView.print(AMOUNT.NONE);
}
this.outputView.print(HEADER.BENEFITS_LIST);
const discountArr = event.getDiscountList();
const giveAwayPrizePrice = event.validateGiveAwayEvent(totalPricePreDiscount)
? menusData.get(MENU.CHAMPAGNE).price
: AMOUNT.ZERO;
this.outputView.printDiscountList([...discountArr, giveAwayPrizePrice], date);
this.outputView.print(HEADER.TOTAL_BENEFITS_PRICE);
const totalBenefits = event.getTotalBenefits(totalPricePreDiscount);
this.outputView.printMoney(totalBenefits, DIVIDER_HYPHEN);
this.outputView.print(HEADER.TOTAL_PRICE_AFTER_DISCOUNT);
const totalDiscount = event.getTotalDiscount();
this.outputView.printMoney(totalPricePreDiscount - totalDiscount);
this.outputView.print(HEADER.EVENT_BADGE);
const badge = event.getBadge(totalBenefits);
this.outputView.print(badge);
return;

Choose a reason for hiding this comment

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

클래스를 분리하는 것의 연습이 미션의 주된 목표였으니
할인을 적용받는 로직 , 적용받은 할인을 print해주는 로직 으로
(예를들어, Discount 클래스 파일, DiscountPrint클래스 파일)
분리해 보는 것은 어떨까요??

}
async #getDateInput() {
this.outputView.print(ASK.SCHEDULE);
const dateInput = await this.inputView.readData(ASK.SCHEDULE);
this.outputView.print(dateInput);
new Date(dateInput);
return dateInput;
}
async #getMenuInput() {
this.outputView.print(ASK.MENU_INFO);
const menuInput = await this.inputView.readData(ASK.MENU_INFO);
this.outputView.print(menuInput);
new Menu(menuInput);
return menuInput;
}
}

export default App;
17 changes: 17 additions & 0 deletions src/ErrorCases.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { ERROR } from './constants';
class DateError extends Error {
constructor(...params) {
super(...params);
this.message = ERROR.message.NOT_VALID_NUMBER;
this.name = ERROR.name.DATE;
}
}
class MenuError extends Error {
constructor(...params) {
super(...params);
this.message = ERROR.message.NOT_VALID_ORDER;
this.name = ERROR.name.ORDER;
}
}

export { DateError, MenuError };
7 changes: 0 additions & 7 deletions src/InputView.js

This file was deleted.

55 changes: 55 additions & 0 deletions src/Model/Date.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { FIRST_DAY_OF_MONTH, LAST_DAY_OF_MONTH, EVENT_DAYS } from '../constants.js';
import { DateError } from '../ErrorCases.js';
class Date {
#date;
constructor(string) {
this.#date = this.#validateDate(string);
this.getDate = this.getDate;
this.getIsWeekend = this.getIsWeekend;
this.getIsStarred = this.getIsStarred;
}
#validateDate(string) {
if (string) {
const number = Number(string);
this.#validateInteger(number);
this.#validateRange(number);
return number;
}
this.#throwError();
}
#validateInteger(number) {
if (Number.isInteger(number)) {
return number;
}
this.#throwError();
}
#validateRange(number) {
if (number >= FIRST_DAY_OF_MONTH && number <= LAST_DAY_OF_MONTH) {
return number;
}
Copy link

Choose a reason for hiding this comment

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

#validateInteget(), #validateRange() 두 함수가 예외 범위가 아닌 경우, number를 return하도록 되어있는데,
#validateDate()에서 이 두 함수의 리턴값을 활용하고 있지 않는 것 같아요

return값을 사용하는 부분이 없다면, 아래와 같이 작성해보는건 어떨까 싶습니다!

#validateInteger(number) {
  if (!Number.isInteger(number)) {
    this.#throwError();
  }

this.#throwError();
}
#validateIfWeekend(date) {
if (EVENT_DAYS.WEEKENDS.includes(date)) {
return true;
}
return false;
}
getDate() {
return this.#date;
}
getIsWeekend() {
return this.#validateIfWeekend(this.#date);
}
getIsStarred(date) {
if (EVENT_DAYS.STARRED_DAYS.includes(date.getDate())) {
return true;
}
return false;
}
#throwError() {
throw new DateError();
}
}

export default Date;
127 changes: 127 additions & 0 deletions src/Model/Event.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import {
EVENT_BADGE,
AMOUNT,
D_DAY_EVENT_STARTING_PRICE,
D_DAY_EVENT_INCREASING_PRICE,
STARRED_DAY_EVENT_DISCOUNT,
WEEK_EVENT_DISCOUNT,
COURSE,
MENU,
CHRISTMAS_D_DAY,
GIVEAWAY_EVENT_MIN_PRICE,
} from '../constants.js';
import menusData from '../menus.js';

class Event {
#discount;
constructor(date, menu) {
this.#discount = [
this.#calculateDDayEventDiscount(date),
this.#calculateWeekEventDiscount(date, menu),
this.#calculateStarredDayEventDiscount(date),
];
this.getDiscountList = this.getDiscountList;
this.validateGiveAwayEvent = this.validateGiveAwayEvent;
this.getGiveAwayInfo = this.getGiveAwayInfo;
this.getBadge = this.getBadge;
}
#calculateDDayEventDiscount(date) {
const dDayDate = date.getDate();
if (dDayDate > CHRISTMAS_D_DAY) {
return AMOUNT.ZERO;
}
const dDayDiscount = D_DAY_EVENT_STARTING_PRICE + D_DAY_EVENT_INCREASING_PRICE * (date.getDate() - 1);
return dDayDiscount;
}
#calculateWeekEventDiscount(date, menu) {
const isWeekend = date.getIsWeekend();

if (!isWeekend) {
//dessert
const dessertFoodArr = this.#getDessertFoodArr(menu);
let weekDiscount = 0;
if (dessertFoodArr.length > 0) {
dessertFoodArr.forEach((food) => {
weekDiscount += food.getAmount() * WEEK_EVENT_DISCOUNT;
});
return weekDiscount;
}
return AMOUNT.ZERO;
}
if (isWeekend) {
//main
const mainFoodArr = this.#getMainFoodArr(menu);
let weekDiscount = 0;
if (mainFoodArr.length > 0) {
mainFoodArr.forEach((food) => {
weekDiscount += food.getAmount() * WEEK_EVENT_DISCOUNT;
});
return weekDiscount;
}
return AMOUNT.ZERO;
}
return AMOUNT.ZERO;
}
Copy link

Choose a reason for hiding this comment

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

메소드의 길이가 15자를 넘어가서, dessert관련 계산 로직과 food관련 계산 로직을 따로 분리해보는 건 어떨까 싶어요!
다른 더 좋은 방법이 있다면 그것도 좋을 것 같습니다 :)

#calculateStarredDayEventDiscount(date) {
if (date.getIsStarred(date)) {
return STARRED_DAY_EVENT_DISCOUNT;
}
return AMOUNT.ZERO;
}
#getDessertFoodArr(menu) {
const orderedFoodArr = menu.getFoodArr();
const dessertFoodArr = [];
orderedFoodArr.forEach((orderedFood) => {
const orderedFoodCourse = orderedFood.getCourse();
if (orderedFoodCourse === COURSE.DESSERT) {
dessertFoodArr.push(orderedFood);
}
});
return dessertFoodArr;
}
#getMainFoodArr(menu) {
const orderedFoodArr = menu.getFoodArr();
const mainFoodArr = [];
orderedFoodArr.forEach((orderedFood) => {
const orderedFoodCourse = orderedFood.getCourse();
if (orderedFoodCourse === COURSE.MAIN) {
mainFoodArr.push(orderedFood);
}
});
return mainFoodArr;
}
validateGiveAwayEvent(totalPricePreDiscount) {
if (totalPricePreDiscount >= GIVEAWAY_EVENT_MIN_PRICE) {
return true;
}
return false;
}
getTotalBenefits(totalPricePreDiscount) {
if (this.validateGiveAwayEvent(totalPricePreDiscount)) {
return this.#discount[0] + this.#discount[1] + this.#discount[2] + menusData.get(MENU.CHAMPAGNE).price;
}
return this.#discount[0] + this.#discount[1] + this.#discount[2] + AMOUNT.ZERO;
}
getDiscountList() {
return this.#discount;
}
getTotalDiscount() {
return this.#discount[0] + this.#discount[1] + this.#discount[2];
}
getGiveAwayInfo() {
return [MENU.CHAMPAGNE, AMOUNT.ONE];
}
getBadge(totalBenefits) {
if (totalBenefits >= EVENT_BADGE.SANTA_MIN) {
return EVENT_BADGE.SANTA;
}
if (totalBenefits >= EVENT_BADGE.TREE_MIN) {
return EVENT_BADGE.TREE;
}
if (totalBenefits >= EVENT_BADGE.STAR_MIN) {
return EVENT_BADGE.STAR;
}
return AMOUNT.NONE;
}
}
export default Event;
Loading