Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough매력포인트에 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt (1)
255-268: 행 분할이 문자열 순서와 3개 고정 가정에 너무 묶여 있습니다.지금은 6개라 맞아 보이지만,
novel_rating_charm_points순서가 바뀌거나 항목이 하나만 더 늘어도 두 번째 row에 전부 몰립니다. 게다가 XML 쪽 두 row가 모두singleLine=true라 이후 칩이 잘릴 가능성이 큽니다. 표시 순서는 타입 안전한 목록으로 두고, 행은chunked(3)처럼 계산하는 쪽이 안전합니다.♻️ 예시
- val firstRow = charmPoints.take(3) - val secondRow = charmPoints.drop(3) + val rows = charmPoints.chunked(3) binding.wcgNovelRatingCharmPointsRow1.removeAllViews() binding.wcgNovelRatingCharmPointsRow2.removeAllViews() - firstRow.forEach { charmPoint -> + rows.getOrNull(0).orEmpty().forEach { charmPoint -> binding.wcgNovelRatingCharmPointsRow1.addChip(createCharmPointChip(charmPoint)) } - secondRow.forEach { charmPoint -> + rows.getOrNull(1).orEmpty().forEach { charmPoint -> binding.wcgNovelRatingCharmPointsRow2.addChip(createCharmPointChip(charmPoint)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt` around lines 255 - 268, 현재 NovelRatingActivity에서 charmPoints를 고정 2행(take(3)/drop(3))으로 나누고 있어 항목 수나 순서 변화에 취약합니다; charmPoints = getString(novel_rating_charm_points).toWrappedCharmPoint()로 받은 리스트를 고정 가정 대신 charmPoints.chunked(3)로 행 단위로 분할하고 각 행(chunk)을 순회하면서 createCharmPointChip을 사용해 칩을 추가하세요; 기존 binding.wcgNovelRatingCharmPointsRow1/Row2는 먼저 removeAllViews()로 비운 뒤 chunk 인덱스에 따라 0이면 Row1, 1이면 Row2에 추가하고 더 많은 chunk가 나오면 새 ChipGroup(또는 XML에서 미리 준비한 추가 행)을 인플레이트하거나 동적으로 생성해 추가하도록 변경해 대처하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt`:
- Around line 255-268: 현재 NovelRatingActivity에서 charmPoints를 고정
2행(take(3)/drop(3))으로 나누고 있어 항목 수나 순서 변화에 취약합니다; charmPoints =
getString(novel_rating_charm_points).toWrappedCharmPoint()로 받은 리스트를 고정 가정 대신
charmPoints.chunked(3)로 행 단위로 분할하고 각 행(chunk)을 순회하면서 createCharmPointChip을 사용해
칩을 추가하세요; 기존 binding.wcgNovelRatingCharmPointsRow1/Row2는 먼저 removeAllViews()로 비운
뒤 chunk 인덱스에 따라 0이면 Row1, 1이면 Row2에 추가하고 더 많은 chunk가 나오면 새 ChipGroup(또는 XML에서 미리
준비한 추가 행)을 인플레이트하거나 동적으로 생성해 추가하도록 변경해 대처하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8196f223-0560-4242-99c7-114ed65fdeef
📒 Files selected for processing (4)
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.ktapp/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.ktapp/src/main/res/layout/activity_novel_rating.xmlcore/resource/src/main/res/values/strings.xml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt (1)
15-15:fromString()은 표시 문자열까지 함께 받게 해두는 편이 안전합니다.이 함수는 API 응답 문자열을 바로 받아 변환하는 진입점인데, 현재는 enum name 형태만 매칭해서 한글 라벨이 들어오면 호출부의
mapNotNull에서 조용히 누락됩니다. 같은 개념이 다른 모델에서는 값/표시명을 따로 들고 있으니, 여기서도korean까지 허용해 두면 포맷 변화에 더 견고합니다.예시 수정안
- fun fromString(value: String): AttractivePoints? = entries.find { it.name.equals(value, ignoreCase = true) } + fun fromString(value: String): AttractivePoints? = + entries.find { point -> + point.name.equals(value, ignoreCase = true) || point.korean == value + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt` at line 15, fromString currently only matches enum constant names so API responses containing the Korean label get dropped; update AttractivePoints.fromString to also compare the incoming value against the enum's display label (e.g., the korean property) in a case-insensitive/trimmed way so it returns the correct AttractivePoints for either the enum name or its korean label (keep the signature AttractivePoints?.fromString(value: String) and search entries by name.equals(value, ignoreCase = true) || korean.equals(value, ignoreCase = true)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt`:
- Line 15: fromString currently only matches enum constant names so API
responses containing the Korean label get dropped; update
AttractivePoints.fromString to also compare the incoming value against the
enum's display label (e.g., the korean property) in a case-insensitive/trimmed
way so it returns the correct AttractivePoints for either the enum name or its
korean label (keep the signature AttractivePoints?.fromString(value: String) and
search entries by name.equals(value, ignoreCase = true) || korean.equals(value,
ignoreCase = true)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b6c62ce-8e33-4c8b-9e6a-136a2bf73b9b
📒 Files selected for processing (1)
app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt
| chip.isSelected = previousSelectedCharmPoints.contains( | ||
| charmPoints.find { charmPoint -> charmPoint.title == chip.text.toString() }, | ||
| ) | ||
| val selectedTitles = previousSelectedCharmPoints.map { it.title }.toSet() |
Contributor
There was a problem hiding this comment.
previousSelectedCharmPoints가 이미 unique한 값들을 가지며 개수가 적다면 toList가 더 빠른 것으로 알고있습니다!
그리고 데이터가 많다면 mapTo(HashSet()) 구문을 써보는 것도 고려하시면 좋을 것 같아요!
Contributor
Author
There was a problem hiding this comment.
이번 케이스는 selectedTitles를 만든 뒤 각 칩에 대해 포함 여부를 여러 번 확인하는 구조라, 생성 비용보다 조회 비용을 줄이는 쪽이 더 적절하다고 판단해 Set은 유지했습니다.
또 mapTo(hashSetOf()) 방식도 생각해봤지만, 현재 데이터 크기가 작고 매력포인트 특성상 크게 확장될 가능성은 낮아 보여 우선은 가독성을 고려해 map { }.toSet() 형태로 유지했습니다.
말씀 주신 덕분에 생성 비용 관점도 함께 생각해볼 수 있었습니다. 감사합니다!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt`:
- Line 255: The charm-point parsing can return an empty list (via
toWrappedCharmPoint()) and wipe the UI; update the caller in NovelRatingActivity
(where val charmPoints =
getString(novel_rating_charm_points).toWrappedCharmPoint() and similarly for the
other two lines) to handle an empty result by falling back to a safe default
(e.g., keep the raw split tokens or a single "unknown" charm point) instead of
applying an empty list to the chip UI, or alternatively adjust
toWrappedCharmPoint() so it never returns an empty list on parse failure but
returns the original tokens or a sensible fallback; ensure the fix is applied
for the three occurrences currently using toWrappedCharmPoint().
- Around line 220-223: The current forced cast in the chipGroup.forEach block
(view as WebsosoChip) can throw ClassCastException if a non-WebsosoChip view is
present; change this to a safe check or filtering: either iterate only over
WebsosoChip instances using chipGroup.filterIsInstance<WebsosoChip>() or use a
safe cast like val chip = view as? WebsosoChip ?: return@forEach and then set
chip.isSelected = chip.text.toString() in selectedTitles so non-chip views are
skipped safely (locate the code in NovelRatingActivity.kt inside the
chipGroup.forEach block).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2bb16af3-d2c1-4057-ab42-7fb6b02c03d5
📒 Files selected for processing (5)
app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.ktapp/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.ktapp/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.ktapp/src/main/res/layout/activity_novel_rating.xmlcore/resource/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt
- app/src/main/res/layout/activity_novel_rating.xml
Comment on lines
+220
to
+223
| chipGroup.forEach { view -> | ||
| val chip = view as WebsosoChip | ||
| chip.isSelected = chip.text.toString() in selectedTitles | ||
| } |
There was a problem hiding this comment.
칩 순회 중 강제 캐스팅은 런타임 크래시 위험이 있습니다.
view as WebsosoChip는 추후 레이아웃 변경으로 다른 View가 섞이면 ClassCastException을 유발할 수 있습니다.
🔧 제안 수정안
).forEach { chipGroup ->
chipGroup.forEach { view ->
- val chip = view as WebsosoChip
- chip.isSelected = chip.text.toString() in selectedTitles
+ (view as? WebsosoChip)?.let { chip ->
+ chip.isSelected = chip.text.toString() in selectedTitles
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt`
around lines 220 - 223, The current forced cast in the chipGroup.forEach block
(view as WebsosoChip) can throw ClassCastException if a non-WebsosoChip view is
present; change this to a safe check or filtering: either iterate only over
WebsosoChip instances using chipGroup.filterIsInstance<WebsosoChip>() or use a
safe cast like val chip = view as? WebsosoChip ?: return@forEach and then set
chip.isSelected = chip.text.toString() in selectedTitles so non-chip views are
skipped safely (locate the code in NovelRatingActivity.kt inside the
chipGroup.forEach block).
Contributor
Author
There was a problem hiding this comment.
현재 구조에서는 chipGroup에 createCharmPointChip()으로 생성한 WebsosoChip만 추가된다는 전제로 작성해서 강제 캐스팅을 사용했습니다.
추후 다른 View가 섞일 가능성을 열어두는 방향이라면 방어적으로 처리할 수도 있겠지만, 현재는 잘못된 타입이 들어오는 경우 조용히 넘기기보다 빠르게 드러나는 편이 낫다고 판단했습니다.
There was a problem hiding this comment.
@devfeijoa, 납득됩니다. setupCharmPointChips()에서 createCharmPointChip()을 통해서만 칩이 추가되므로 해당 불변식이 코드 구조적으로 보장되어 있고, 잘못된 타입이 유입될 경우 조용히 넘기는 것보다 빠르게 크래시로 드러나게 하는 fail-fast 전략은 타당한 판단입니다. 현재 구조에서는 강제 캐스팅이 적절합니다.
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.kt (1)
23-28: 에러 처리 방식 불일치.
toWrappedCharmPoint()는 알 수 없는 값에 대해 예외를 던지지만,toFormattedCharmPoint()는 빈 문자열을 반환합니다. 동일한 enum 내에서 일관된 에러 처리 전략을 사용하는 것이 유지보수에 유리합니다.♻️ 일관성 개선 제안
fun String.toFormattedCharmPoint(): String { - entries.forEach { charmPoint -> - if (charmPoint.value == this) return charmPoint.title - } - return "" + return entries.find { it.value == this }?.title ?: "" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.kt` around lines 23 - 28, toFormattedCharmPoint() currently returns an empty string for unknown values while toWrappedCharmPoint() throws an exception; change toFormattedCharmPoint() to mirror the enum's consistent error handling by throwing the same exception type (e.g., IllegalArgumentException) when no matching charmPoint is found, and include the input string in the error message so it matches the behavior of toWrappedCharmPoint() and makes failures explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.kt`:
- Around line 16-21: The new toWrappedCharmPoint implementation throws
IllegalArgumentException for unknown titles causing potential runtime crashes
and also reports the untrimmed rawTitle in the error; change it to safely handle
unknown values by either filtering them out (return only matching entries) or,
if you keep throwing, trim the value used in the message and include
trimmedTitle in the error; update the logic in toWrappedCharmPoint to use
entries.find { charmPoint -> charmPoint.title == trimmedTitle } ?: (either drop
the item or throw IllegalArgumentException("존재하지 않는 매력포인트입니다: $trimmedTitle"))
so the behavior is null-safe and the error message matches the comparison.
---
Nitpick comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.kt`:
- Around line 23-28: toFormattedCharmPoint() currently returns an empty string
for unknown values while toWrappedCharmPoint() throws an exception; change
toFormattedCharmPoint() to mirror the enum's consistent error handling by
throwing the same exception type (e.g., IllegalArgumentException) when no
matching charmPoint is found, and include the input string in the error message
so it matches the behavior of toWrappedCharmPoint() and makes failures explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6372ea41-9991-4e25-a1da-25cf1c847f13
📒 Files selected for processing (1)
app/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
Screen_recording_20260310_182431.mp4
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
작품 평가 화면의 매력포인트 부분을 '필력'을 추가 하면서 2줄 구조로 변경했습니다.
기존 방식은 줄바꿈 시 각 줄이 start 정렬되어 피그마와 다르게 보였고, 이번에는 각 row를 별도 그룹으로 분리해 가운데 정렬되도록 수정했습니다.
따라서 매력포인트 칩 영역을 row1, row2 두 개의 WebsosoChipGroup으로 분리하고, 이를 LinearLayout으로 감싸center_horizontal 정렬되도록 변경했습니다.
이를 통해 각 줄이 독립적으로 가운데 정렬된 형태로 보이도록 맞췄습니다.
추가로 필력이 노출/매핑되도록 novel_rating_charm_points와 CharmPoint enum을 함께 업데이트했습니다.
추가로, 기존 선택 상태를 갱신하던 로직은 단일 wcgNovelRatingCharmPoints 기준이었는데, row1/row2 구조로 변경되면서 두 그룹을 모두 순회하도록 수정했습니다.
작품 평가 화면과 마이페이지가 매력포인트를 각각 다른 enum/매핑으로 관리하고 있어, 필력 값이 마이페이지에서 누락되고 있었습니다. 따라서 해당 매핑도 함께 추가했습니다.
서재 필터링에도 필력 아이콘 추가하였습니다.
Summary by CodeRabbit
New Features
Refactor
UI 변경