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

feature/define cafe order system #12

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

Gangsss
Copy link
Collaborator

@Gangsss Gangsss commented Jul 31, 2022

Changes?

데코레이터 패턴을 위한 코드 추가

Why we need?

데코레이터 패턴 학습

Test?

cafe_order_test.py를 통해 테스트

Checklist

  • Assignees updated the README, if necessary.
  • Assignees checked the runnability of the code.
  • Reviewers checked the runnability of the code.

Anything Else? (Optional)

없음

@github-actions github-actions bot added the size/L 110+ lines are updated label Jul 31, 2022
@github-actions github-actions bot added the feature Some features are implemented label Jul 31, 2022
@Gangsss Gangsss changed the title featue/define beverage class feature/define cafe order system Jul 31, 2022
@Gangsss Gangsss marked this pull request as draft July 31, 2022 12:42
다양한 Beverage들을 구현하기 위한 Abstractive class
"""

def getdescription(self) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def getdescription(self) -> str:
def get_description(self) -> str:

으로 끊어주면 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 피드백 감사합니다

Comment on lines 33 to 39

Args:
None

Returns:
None
"""
Copy link
Member

Choose a reason for hiding this comment

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

input이나 return이 없는 경우, 작성하지 않는 것을 표준으로 하겠습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 확인했습니다 다음 커밋 시 반영하겠습니다.



class HouseBlend(Beverage):
"""summary
Copy link
Member

Choose a reason for hiding this comment

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

summary 단어 제거 부탁합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 알겠습니다!

@Gangsss
Copy link
Collaborator Author

Gangsss commented Sep 21, 2022

Thx for your comment @Kimdongui

Gangsss and others added 3 commits September 26, 2022 14:16
Co-authored-by: Kim dong hyun, 김동현 <rkdqus2006@naver.com>
@Gangsss Gangsss requested a review from Kimdongui September 26, 2022 08:56
@Gangsss Gangsss marked this pull request as ready for review September 26, 2022 11:58
@jonhyuk0922 jonhyuk0922 self-requested a review September 26, 2022 13:14
Comment on lines +62 to +63
Args:
None
Copy link
Collaborator

Choose a reason for hiding this comment

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

이부분 제거되어도 좋을듯합니다!

Comment on lines +100 to +105
if self._beverage.get_size() == "tall":
beverage_cost += 0.10
elif self._beverage.get_size() == "grande":
beverage_cost += 0.15
elif self._beverage.get_size() == "venti":
beverage_cost += 0.20
Copy link
Collaborator

Choose a reason for hiding this comment

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

이부분 딕셔너리를 활용하는 건 별로일까요..?

설명을 덧붙이는 함수

Returns:
beverage의 이름 + ", 모카"
Copy link
Member

Choose a reason for hiding this comment

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

type도 같이 명시 하면 좋을 것 같습니다!



def main() -> None:
"""summary
Copy link
Member

Choose a reason for hiding this comment

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

제거!

from abc import ABCMeta, abstractmethod


class Beverage(metaclass=ABCMeta):
Copy link
Collaborator

@jonhyuk0922 jonhyuk0922 Sep 26, 2022

Choose a reason for hiding this comment

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

https://docs.python.org/ko/3/library/abc.html
: abstractmethod 이 데코레이터를 사용하려면 클래스의 메타 클래스가 [ABCMeta]이거나 여기에서 파생된 것이어야 합니다.

라고 공식문서에 적혀있습니다.

def __init__(self):
self.size = "tall"

def get_description(self) -> str:
Copy link
Collaborator

@jonhyuk0922 jonhyuk0922 Sep 26, 2022

Choose a reason for hiding this comment

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

경현 said : 해당 메서드도 abstractmethod 데코레이터가 필요할 것 같습니다.

self._beverage = bevarage

@property
def beverage(self) -> Beverage:
Copy link
Collaborator

@jonhyuk0922 jonhyuk0922 Sep 26, 2022

Choose a reason for hiding this comment

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

추상클래스에서 propertyabstractmethod 데코레이터로 감싸진 메서드는 재정의되야 인스턴스를 생성할 수 있다고 알고있는데, CondimentDecorator를 상속받은 Mocha에서 beverage를 재정의안해주는데 인스턴스가 생성될까요?

https://docs.python.org/ko/3/library/abc.html

Copy link
Member

Choose a reason for hiding this comment

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

검색 키워드 : 프로퍼티

Description:
다양한 Beverage들을 구현하기 위한 Abstractive class
"""

Copy link
Collaborator

@KyungHyunLim KyungHyunLim Sep 19, 2022

Choose a reason for hiding this comment

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

get_description은 abstract 메소드가 안붙어있는 이유가 있나요?

Comment on lines +93 to +96
Description:
cost 계산시 size에 따라서 cost를 계산해준다.
Returns:
beverage cost + size depends cost
Copy link
Collaborator

Choose a reason for hiding this comment

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

Description과 Returns 사이에 공백 있고 없고 통일하면 좋을 것 같습니다.

@Kimdongui
Copy link
Member

test.pytests/ 디렉토리 밑으로 옮겨주세요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Some features are implemented size/L 110+ lines are updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants