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

Feat:[공통] Pagination 마지막 페이지 체크 로직 보완 #85 #88

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

juan-rybczinski
Copy link
Contributor

@juan-rybczinski juan-rybczinski commented Feb 11, 2024

Description

  • Pagination API에 추가된 is_last_page을 적용했습니다.
  • 개별 PaginationResponse가 기본 PaginationResponse를 extends 하도록 변경했습니다.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@juan-rybczinski juan-rybczinski added the review needed 개발자가 리뷰어에게 코드 리뷰를 요청 label Feb 11, 2024
@juan-rybczinski juan-rybczinski self-assigned this Feb 11, 2024
@juan-rybczinski juan-rybczinski linked an issue Feb 11, 2024 that may be closed by this pull request
Comment on lines +5 to +15
class BreedsPaginationResponse extends PaginationResponse<Breed> {
BreedsPaginationResponse({
required this.page,
required this.size,
required this.items,
required super.page,
required super.size,
required super.items,
required super.isLastPage,
});

BreedsPaginationResponse.fromJson(
Map<String, dynamic> json,
) : page = json['page'] as int,
size = json['size'] as int,
items = (json['items'] as List<dynamic>)
.map((e) => Breed.fromJson(e))
.toList();

@override
final int page;

@override
final int size;

@override
final List<Breed> items;

@override
bool operator ==(Object other) {
if (identical(this, other)) return true;

return other is BreedsPaginationResponse &&
other.page == page &&
other.size == size;
}

@override
int get hashCode => page.hashCode ^ size.hashCode;

@override
Map<String, dynamic> toJson() {
// TODO: implement toJson
throw UnimplementedError();
}
) : super.fromJson(json, (e) => Breed.fromJson(e));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

개별 PaginationResponse들이 기본 PaginationResponse들을 기존에 implements 하던 것을 extends 하도록 변경해 불필요하게 반복되는 로직들을 제거했습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

PaginationResponse가 다른 클래스들의 순수한 is-a 관계인 것 같아 상속으로 바꿔도 좋을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

불필요하게 반복되는 코드를 줄여보자는 관점으로 접근 했었는데 is-ahas-aextendsimplements를 판단할 수 있군요.

}
bool operator ==(Object other) =>
identical(this, other) ||
other is PaginationResponse &&
Copy link
Contributor

Choose a reason for hiding this comment

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

요 비교 로직에서 other is PaginationResponse<T> 로 비교하지 않아도 괜찮을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

별 생각없이 IDE 자동 완성 기능으로 작성한 부분이었는데 Generic 타입이 달라도 같은 값이라고 판단하고 있었네요.

f827a27

Comment on lines +5 to +15
class BreedsPaginationResponse extends PaginationResponse<Breed> {
BreedsPaginationResponse({
required this.page,
required this.size,
required this.items,
required super.page,
required super.size,
required super.items,
required super.isLastPage,
});

BreedsPaginationResponse.fromJson(
Map<String, dynamic> json,
) : page = json['page'] as int,
size = json['size'] as int,
items = (json['items'] as List<dynamic>)
.map((e) => Breed.fromJson(e))
.toList();

@override
final int page;

@override
final int size;

@override
final List<Breed> items;

@override
bool operator ==(Object other) {
if (identical(this, other)) return true;

return other is BreedsPaginationResponse &&
other.page == page &&
other.size == size;
}

@override
int get hashCode => page.hashCode ^ size.hashCode;

@override
Map<String, dynamic> toJson() {
// TODO: implement toJson
throw UnimplementedError();
}
) : super.fromJson(json, (e) => Breed.fromJson(e));
Copy link
Contributor

Choose a reason for hiding this comment

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

PaginationResponse가 다른 클래스들의 순수한 is-a 관계인 것 같아 상속으로 바꿔도 좋을 것 같습니다!

@Yellowtoast Yellowtoast added merge needed 리뷰어가 코드 리뷰를 마치고 개발자에게 머지를 요청 answer needed 리뷰어가 개발자에게 코드 리뷰에 대한 응답을 요청 and removed review needed 개발자가 리뷰어에게 코드 리뷰를 요청 merge needed 리뷰어가 코드 리뷰를 마치고 개발자에게 머지를 요청 labels Feb 12, 2024
- is_last_page 적용
- 개별 PaginationResponse가 기본 PaginationResponse를 implements -> extends 하도록 변경
@juan-rybczinski juan-rybczinski added merge needed 리뷰어가 코드 리뷰를 마치고 개발자에게 머지를 요청 review needed 개발자가 리뷰어에게 코드 리뷰를 요청 and removed answer needed 리뷰어가 개발자에게 코드 리뷰에 대한 응답을 요청 merge needed 리뷰어가 코드 리뷰를 마치고 개발자에게 머지를 요청 labels Feb 15, 2024
@Yellowtoast Yellowtoast added the merge needed 리뷰어가 코드 리뷰를 마치고 개발자에게 머지를 요청 label Feb 20, 2024
Copy link
Contributor

@Yellowtoast Yellowtoast left a comment

Choose a reason for hiding this comment

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

좋은 코드로 고쳐주셔서 감사합니다! 페이지네이션을 구현해주신 덕에 저도 돌봄급구 리스트를 금방 구현할 수 있었네요. 감사합니다! 👍

@juan-rybczinski juan-rybczinski removed review needed 개발자가 리뷰어에게 코드 리뷰를 요청 merge needed 리뷰어가 코드 리뷰를 마치고 개발자에게 머지를 요청 labels Feb 20, 2024
@juan-rybczinski juan-rybczinski merged commit 8c6ff01 into dev Feb 20, 2024
@juan-rybczinski juan-rybczinski deleted the feature/85-is-last-page branch February 20, 2024 10:50
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.

[공통] Pagination 마지막 페이지 체크 로직 보완
2 participants