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

CompositeBot 클래스를 만듭니다. #35

Closed
wants to merge 4 commits into from
Closed

Conversation

jwChung
Copy link
Member

@jwChung jwChung commented Jul 12, 2016

이 이슈을 내가 구현 한다면 아래 두 가지 방법 중 어떤 것을 택할까에 대해서 고민을 많이 했습니다.

  1. @masunghoon 님이 작성하신 것 중복없는 dice 구현  #34 와 같이 case 문을 추가하는 방법
  2. NonDuplicatedDiceBot 과 같은 새로운 클래스를 만드는 방법

두 가지 방법은 아래와 같은 장단점이 있어서 선택이 쉽지가 않네요.

  1. 장점: 구현이 쉽다, 단점: OCP를 위배함으로 코드가 수정되면 복잡도가 증가
  2. 장점: OCP 를 유지, 단점: 코드조각이 많아져 전체를 한눈에 보기 어렵다

이 PR은 구현은 후자의 방법을 고민한 것입니다. CompositeBot 클래스는 아래와 같이 사용할 수 있습니다.

val bot = new CompositeBot(new DiceBot, new OtherBot)
bot.process("2d4")

@jwChung
Copy link
Member Author

jwChung commented Jul 12, 2016

@masunghoon 생각해보니깐 굳이 작성하신 #34 결부시켜서 고민할 필요없겠네요. 현재의 PR과 그냥 독립적으로 생각해도 되겠네요. 가볍게 리뷰 한번 부탁드립니다.

@naxxster
Copy link

구현된 모습 상으로는 chain of responsibility 에 더 가까운 것 같습니다.
확실히 두 가지 방법 모두 장단이 있는데, 어떻게 보면 지금 처리할 수 있는 객체인지 판단하는 로직과 동작을 처리하는 로직이 가깝게 붙어있어서 문제가 커지는 게 아닐까 싶습니다. 두 기능을 분리하면 조금 더 유연하게 처리할 수 있지 않나 생각이 드는군요.

@younggi
Copy link
Contributor

younggi commented Jul 13, 2016

process method의 실행 결과가 아닌 다른 방법으로 객체를 선택하는 건 어떨까 생각이 들었습니다.
find 로 객체 선택의 경우가 최악이 아닐 경우 모든 process method가 실행되지는 않겠지만요..

@jwChung
Copy link
Member Author

jwChung commented Jul 13, 2016

@naxxster
chain of responsibility 맞습니다. 굳이 따지면 Composite + Chain of responsibility 이라할 수 있겠네요. 뭐 이게 중요한게 아니라...

어떻게 보면 지금 처리할 수 있는 객체인지 판단하는 로직과 동작을 처리하는 로직이 가깝게 붙어있어서 문제가 커지는 게 아닐까 싶습니다. 두 기능을 분리하면 조금 더 유연하게 처리할 수 있지 않나 생각이 드는군요.

의미는 이해가 되는데 어떻게 구현하자는 말씀이신지 잘 모르겠습니다. 좀 더 구체적인 답변 부탁드려도 될까요?

@jwChung
Copy link
Member Author

jwChung commented Jul 13, 2016

@younggi

process method의 실행 결과가 아닌 다른 방법으로 객체를 선택하는 건 어떨까 생각이 들었습니다.

@naxxster 의견과 마찬가지로 의미는 이해가 되는데 실제로 어떻게 구현하자는 의견이신지 잘 이해가 안됩니다. 좀 더 구체적인 답변 부탁드려도 될까요?

@hkjlee109
Copy link
Member

Interesting, Captian.
Although this looks decent to me, it seems a bit extravagant.
Maybe this is due to my ignorance.. if we choose to go this implementation for all the functional blocks in the bot, I would feel like I am learning OOP in Scala. Teach me if I am not getting this correctly. Great idea though.

@jwChung
Copy link
Member Author

jwChung commented Jul 16, 2016

@kwoolytech

Although this looks decent to me, it seems a bit extravagant.

extravagant 라는 의견에 어느 정도 동감합니다. YAGNI를 위배하고 있다는 생각이 드네요. 좀 더 필요한 시점으로 미루는 것도 방법일 듯 합니다. 가령 #34 은 DiceBot 클래스 내에서 case 구문으로 처리합니다. 그런 다음 #15 #25 를 구현 역시 DiceBot 클래스 내 case 구문으로 처리해야할까요, 더 나아가 roll 외 또 다른 명령이 생기면 어떻게 될까요? 이 시점에서는 아마 CompositeBot이 필요하게 되지 않을까 합니다. 너무 이르게 이 클래스를 구현하려 했던 건 아닌가합니다.

if we choose to go this implementation for all the functional blocks in the bot, I would feel like I am learning OOP in Scala.

CompositeBot 을 아래 함수로 표현(더 좋은 FP표현이 있을지도 모르겠지만)하면 왜 좋을까요? FP로 표현하자는 사실보다 왜 라는 질문에 촛점을 맞춰야 한다고 생각합니다. 만약 이 왜라는 질문에 합당한 근거가 있다면 FP표현으로 바꿔야 한다고 생각합니다. CompositeBot 와 아래 compose 함수를 장단점을 비교하면서 천천히 Thinking functionally에 익숙해지는 것은 어떨까요?

def compose(processors: Seq[String=>Option[String]]): String=>Option[String] = ???

@jwChung
Copy link
Member Author

jwChung commented Jul 16, 2016

@kwoolytech

The venerable master Qc Na was walking with his student, Anton. Hoping to prompt the master into a discussion, Anton said "Master, I have heard that objects are a very good thing - is this true?" Qc Na looked pityingly at his student and replied, "Foolish pupil - objects are merely a poor man's closures."

Chastised, Anton took his leave from his master and returned to his cell, intent on studying closures. He carefully read the entire "Lambda: The Ultimate..." series of papers and its cousins, and implemented a small Scheme interpreter with a closure-based object system. He learned much, and looked forward to informing his master of his progress.

On his next walk with Qc Na, Anton attempted to impress his master by saying "Master, I have diligently studied the matter, and now understand that objects are truly a poor man's closures." Qc Na responded by hitting Anton with his stick, saying "When will you learn? Closures are a poor man's object." At that moment, Anton became enlightened.

Anton van Straaten

@jwChung
Copy link
Member Author

jwChung commented Jul 16, 2016

이 PR은 YAGNI를 위배하고 있다는 생각이 듭니다. 닫아 놓겠습니다. :)

@jwChung jwChung closed this Jul 16, 2016
@hkjlee109
Copy link
Member

oh.. composite pattern in FP style, thank you for showing me.
I glad that you suggested to compare CompositeBot and compose.
Honestly, I still don't understand FP.
For example, after seeing your code, I had a thought, what if we see a function as an object.. I got confused if this is a functional thinking or object oriented thinking. 😅

@jwChung
Copy link
Member Author

jwChung commented Jul 17, 2016

저도 헷깔려요. 공부하면서 해결될 부분이 아닌가 기대하고 있습니다.

@jwChung jwChung deleted the composite-bot branch July 17, 2016 06:03
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.

4 participants