Skip to content

[1주차] 3단계 #36

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

Open
wants to merge 27 commits into
base: soojinroh
Choose a base branch
from
Open

[1주차] 3단계 #36

wants to merge 27 commits into from

Conversation

soojinroh
Copy link

No description provided.

에뮬레이터 실행 후 설정이 자동 변경된 사항
@Kiboom Kiboom self-requested a review November 13, 2022 15:23
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.

3주차 과제도 훌륭하게 잘 마무리 해주셨습니다 !! 💯

Provider 개념도 잘 이해해서 적용해주셨고, 이전에 피드백 드린 부분들도 잘 반영되었네요!
코드가 개선되어 가는 것이 보기 좋습니다 ! 👍

tear-off 개념에 대해 피드백을 드렸는데, 한 번 확인 후 머지해주시면 될 것 같아요 😀

PR도 기능 별로 잘 쪼개서 올려주셨네요!
고생 많으셨습니다 !!

? null
: () {
menuCounter.minus();
},
Copy link

Choose a reason for hiding this comment

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

// 1번 방식
IconButton(
  onPressed: () {
    menuCounter.minus();
  },
),

// 2번 방식
IconButton(
  onPressed: menuCounter.minus
),

함수를 인자로 넘기는 방식에는 크게 위의 두 가지 방식이 있습니다 !

수진님께서는 1번 방식을 사용해주셨는데요,
1번처럼 익명 함수로 호출하는 것보다 2번처럼 함수를 직접 인자로 넘기는 것이
성능 및 가독성 측면에서 더욱 유리하고, 권장되는 방식입니다 ! 😀

참고로 2번과 같은 방식을 tear-off라고 부릅니다.
자세한 내용은 아래 영상을 참고해보시기 바래요 !

required storeName,
required storeImage,
}) : _storeName = storeName,
_storeImage = storeImage,
Copy link

Choose a reason for hiding this comment

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

멤버 변수들을 private으로 잘 변경해주셨고,
파라미터 명에도 _을 잘 없애주셨습니다 ! 💯👏


var _menuCount = 1;

int get menuCount => _menuCount;
Copy link

Choose a reason for hiding this comment

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

내부 변수와 외부 변수를 잘 구분하여 선언해주셨네요! 💯👍

var _menuPrice = 18000;
var _totalPrice = format.format(_deliveryPrice + _menuPrice);
const deliveryPrice = 3000;
const menuPrice = 18000;
Copy link

Choose a reason for hiding this comment

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

상수 멤버들을 const로 잘 변경해주셨습니다 👍

icon: Icon(Icons.add),
onPressed: () {
menuCounter.add();
},
Copy link

Choose a reason for hiding this comment

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

여기도 tear-off 방식을 적용해볼 수 있을 것 같네요!

@@ -11,7 +13,8 @@ class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return MaterialApp(
home: CartScreen(),
home: ChangeNotifierProvider(
create: (context) => MenuCounterModel(), child: CartScreen()),
Copy link

Choose a reason for hiding this comment

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

아래처럼 마지막 인자 뒤에 ,를 넣어주면, 더 가독성 좋게 포맷팅이 될 거에요!

home: ChangeNotifierProvider(
  create: (context) => MenuCounterModel(), 
  child: CartScreen(),
),                   ^ 콤마 추가!

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.

위에 코멘트와 더불어서 conflict 발생하는 부분도 해결 후 머지해주시면 될 것 같아요 !
고생 많으셨습니다! 💯👍

@ybin0823 ybin0823 added the conflict conflict 발생하여서 머지 불가 label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict conflict 발생하여서 머지 불가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants