Skip to content

[Step1] 1단계 UI-로직 분리하기 #5

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: malibinyun
Choose a base branch
from

Conversation

malibinYun
Copy link
Member

@malibinYun malibinYun commented Nov 9, 2022

안녕하세요 리뷰어님 :)
플러터 스타일이 생각보다 익숙해지는 데 오래 걸리네요 ㅠ_ㅠ
플러터나 다트의 스타일이 아닌 부분들이 보이시면 많이 짚어주시길 부탁드려요.

플러터 자체가 익숙하지 않아서 어렵게만 느껴지네요..
그래서 그런지 커밋을 나눌 생각도 못하고 이것 저것 코드를 막 쳐보다가 통째로 커밋을 했네요😭
회사에서 안드로이드 개발을 MVP 아키텍처로 하고 있고, 이벤트를 구독하는 형태의 코드를 짜지 않은 지 너무 오래돼서 의도에 맞게 로직을 구성했는지 모르겠네요.

로직을 거의 그대로 옮겼다고 생각했는데, 이상하게 첫 2장의 카드를 맞추고 난 뒤에는 아래 로직이 실행되지 않아서 다음 진행이 되지 않아요..

      _frontCardIndexes.clear();
      _frontCardCount = 0;

이 부분은 코드 코멘트로 달아두겠습니다.
타이밍 이슈인 것 같은데 원인을 파악하는 것이 너무나 어렵네요. 😭😭
이 원인 파악하는데 도움이 필요해서 미리 PR을 올립니다😭

Comment on lines +52 to +57
cardsMatchedStream.add(true);
}
}

_frontCardIndexes.clear();
_frontCardCount = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

cardsMatchedStream.add(true); 를 주석 처리하면 아래 indexed와 count가 초기화 되는 것을 확인했어요.
stream 이벤트로 setState가 먼저 불려서 무언가 무시가 되는 것 같은데 해결 법을 전혀 모르겠네요.

처음 한 벌의 카드를 맞춘 뒤에 카드를 누르면, _frontCardCount는 3으로 초기화가 되지 않음을 확인했어요.

@Kiboom Kiboom self-requested a review November 12, 2022 13:55
Copy link

@Kiboom Kiboom left a comment

Choose a reason for hiding this comment

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

2주차 강의 과제도 고생 많으셨습니다! 👏

별도의 데이터 관리 클래스를 선언하여 UI로부터 잘 분리하여 주셨습니다 💯
특정 데이터를 하위 위젯에 효율적으로 공유할 수 있는 방법과,
StreamControllerStreamBuilder의 올바른 사용법에 대해 코멘트 드렸으니 확인 부탁드려요!

final StreamController cardsMatchedStream = StreamController();

List<String> getRandomCardImages() {
return _randomImageNames.map((e) => e).toList();
Copy link

Choose a reason for hiding this comment

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

여기서는 map의 역할이 특별히 없는 듯 해요!
바로 _randomImageNames를 반환해줘도 무방할 듯 합니다.

];

final List<String> _randomImageNames = [];
final FlipCardCore _flipCardCore = FlipCardCore();
Copy link

Choose a reason for hiding this comment

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

FlipCardCore를 별도의 클래스로 잘 분리해주셨습니다! 👏

다만 FlipCardCore는 데이터를 관리하는 클래스인 만큼, 여러 하위 위젯에서 공통적으로 사용될 가능성이 있어요!

1주차 때 배운 InheritedWidget이나 Provider를 사용하면,
나중에 하위 위젯들과 FlipCardCore를 공유하기 훨씬 쉬워질 것 같네요! 🙂

(시간 되실 때 보너스로 진행해보셔도 좋을 듯 해요! 😀)

body: StreamBuilder(
stream: _flipCardCore.cardsMatchedStream.stream,
builder: (context, snapshot) {
List<String> _randomImageNames = _flipCardCore.getRandomCardImages();
Copy link

Choose a reason for hiding this comment

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

StreamController와 마찬가지로 StreamBuilder를 사용할 때도 제네릭 타입을 명시해주는 것을 권장드려요!

StreamBuilder<bool>(
    stream: _flipCardCore.cardsMatchedStream.stream,
    builder: (context, snapshot) {
        // StreamBuilder의 제네릭 타입을 명시해주면, `snapshot.data`의 타입도 명확해집니다.
    },
)

final List<int> _frontCardIndexes = [];

final StreamController toggleCardToFrontStream = StreamController();
final StreamController cardsMatchedStream = StreamController();
Copy link

Choose a reason for hiding this comment

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

StreamController를 사용할 때는 제네릭 타입을 명시해주는 것을 권장드려요!

final StreamController<bool> cardsMatchedStream = StreamController();


@override
void initState() {
super.initState();

print('initState');
_flipCardCore.toggleCardToFrontStream.stream.listen((_) {
_toggleCardToFront();
Copy link

Choose a reason for hiding this comment

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

카드가 2장이 뒤집혔을 때, toggleCardToFrontStream에 이벤트가 발생하는데요,
이벤트로 인해 _toggleCardToFront이 호출 되면,
_flipCardCount에는 어떤 영향을 미치는 지 확인해보시면 좋을 듯 합니다.

첫번째 매칭 후 카드가 더 뒤집히지 않는 이슈와도 연관이 있을 듯 해요!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants