-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] Literals 선언 및 적용 #204
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
Conversation
|
Caution Review failedThe pull request is closed. Assessment against linked issues
Suggested reviewers
Poem
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (13)
Wable-iOS/Presentation/Viewit/Create/View/Subview/ViewitInputView.swift (1)
51-54: Consider consistent constructor formatting.The centralized literal usage is excellent. For consistency, consider formatting the constructor on a single line like the
urlTextFieldabove, unless the line length necessitates the multi-line approach.- let descriptionTextField = UITextField( - pretendard: .body4, - placeholder: StringLiterals.Viewit.descriptionPlaceholder - ).then { + let descriptionTextField = UITextField(pretendard: .body4, placeholder: StringLiterals.Viewit.descriptionPlaceholder).then {Wable-iOS/Core/Literals/String/StringLiterals+Ban.swift (1)
14-16: Consider internationalization for user-facing text.The implementation is clean and follows good centralization practices. However, the hardcoded Korean text may pose challenges for internationalization if the app needs to support multiple languages in the future.
Consider using a localization system for user-facing strings:
- static let sheetMessage = "1회 누적 - 게시글 블라인드 처리\n2회 누적 - 게시글/댓글 블라인드 처리\n3회 누적 - 게시글 작성 제한\n4회 누적 - 계정 정지" + static let sheetMessage = NSLocalizedString("ban.sheet.message", comment: "Ban escalation policy message")Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift (1)
434-434: LGTM! Consistent centralization of ban message.Both methods now correctly use the centralized
StringLiterals.Ban.sheetMessage, ensuring consistent messaging across content and comment ban actions.Consider removing the unused
Constant.Banenum since thesheetMessageis now centralized:enum Ban { static let title = "밴하기" - // sheetMessage is no longer needed here }Also applies to: 448-448
Wable-iOS/Core/Literals/String/StringLiterals+Update.swift (1)
14-17: Well-structured update literals with internationalization considerations.The implementation follows the established pattern for centralized string literals. The update notification constants are well-organized and will improve consistency across the app.
Similar to the Ban literals, consider internationalization for these user-facing strings:
enum Update { - static let title = "업데이트" - static let message = "와블이 새롭게 업그레이드 되었어요.\n업데이트를 통해 최신 버전으로 즐겨보세요." + static let title = NSLocalizedString("update.title", comment: "Update dialog title") + static let message = NSLocalizedString("update.message", comment: "Update dialog message") }Wable-iOS/Core/Literals/Integer/IntegerLiterals.swift (1)
10-15: Well-organized integer constants with room for documentation improvement.The centralized integer literals are well-structured and the values appear reasonable for pagination purposes. The
homeContentCountPerPage = 20aligns with the PR objective to correct the pagination standard.Consider adding documentation comments to explain the purpose of each constant:
enum IntegerLiterals { + /// Initial cursor value indicating no previous pagination cursor static let initialCursor = -1 + /// Number of content items per page for home screen static let homeContentCountPerPage = 20 + /// Default pagination count for general use static let defaultCountPerPage = 15 + /// Number of comments per page in profile views static let profileCommentCountPerPage = 10 }Wable-iOS/Core/Literals/String/StringLiterals+URL.swift (2)
14-21: Consider renaming the nested enumURLto avoid shadowing and improve clarity
While theStringLiterals.URLnamespace is clear in context, naming the enumURLmay cause confusion with the FoundationURLtype, especially in code completion or imports. A more descriptive name likeURLsorLinkmight improve readability.
18-18: Nitpick name consistency forlittlyconstant
The property namelittlyreflects the domain but lacks context (e.g.,littlyURLorinviteLink). Consider renaming for clarity and to convey its purpose.Wable-iOS/Presentation/Community/View/CommunityViewController.swift (3)
201-203: Centralize action button titles for localization consistency
You’ve replaced sheet titles/messages with literals, but the action button titles ("취소","신청하기", etc.) are still hardcoded. Consider adding these toStringLiterals.Community(or a newStringLiterals.Action) for consistent localization.
226-228: Centralize action button titles for localization consistency
Similar to above, the button titles"나중에"and"푸시알람 설정"are hardcoded. Centralizing these inStringLiteralswill make localization easier and ensure consistency.
275-277: Enhance error handling for invalid URLs
Currently, iffeedbackFormis malformed, the tap silently does nothing. Consider logging an error or showing an alert when URL parsing fails to aid debugging and improve user feedback.Wable-iOS/Core/Literals/String/StringLiterals+Community.swift (1)
14-28: Avoid usingdescriptionas a property name
Thedescriptionidentifier can shadow Swift’sCustomStringConvertibleprotocol member, which might lead to confusion. Consider renaming it tocommunityDescriptionfor clarity.Wable-iOS/Core/Literals/String/StringLiterals+Profile.swift (2)
15-19: Enhance readability with Swift multiline string literalsFor multi-line or long strings (e.g.,
withdrawalReasonSubtitleandwithdrawalGuideTitle), consider using Swift’s multiline string syntax (""" ... """) to avoid manual\nescapes and improve readability:-static let withdrawalReasonSubtitle = "계정을 삭제하시려는 이유를 말해주세요\n서비스 개선에 중요한 자료로 활용하겠습니다" +static let withdrawalReasonSubtitle = """ +계정을 삭제하시려는 이유를 말해주세요 +서비스 개선에 중요한 자료로 활용하겠습니다 +""" -static let withdrawalGuideTitle = "계정을 삭제하기 전,\n아래 내용을 꼭 확인해 주세요" +static let withdrawalGuideTitle = """ +계정을 삭제하기 전, +아래 내용을 꼭 확인해 주세요 +"""
10-27: Clarify access control for shared literalsThese constants default to
internal. If they need to be consumed by other modules, feature targets, or unit tests, explicitly mark the extension, enum, and static properties aspublicto guarantee visibility across your codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
Wable-iOS.xcodeproj/project.pbxproj(13 hunks)Wable-iOS/App/SceneDelegate.swift(2 hunks)Wable-iOS/Core/Literals/Integer/IntegerLiterals.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Ban.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Community.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+NotFound.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Overview.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Profile.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+URL.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Update.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Viewit.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals.swift(1 hunks)Wable-iOS/Presentation/Community/View/CommunityRegisterCompleteViewController.swift(1 hunks)Wable-iOS/Presentation/Community/View/CommunityViewController.swift(5 hunks)Wable-iOS/Presentation/Home/ViewModel/HomeDetailViewModel.swift(1 hunks)Wable-iOS/Presentation/Home/ViewModel/HomeViewModel.swift(2 hunks)Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift(1 hunks)Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift(2 hunks)Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotificationViewModel.swift(2 hunks)Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift(1 hunks)Wable-iOS/Presentation/Overview/Detail/View/AnnouncementDetailViewController.swift(1 hunks)Wable-iOS/Presentation/Overview/Detail/View/Subview/AnnouncementDetailView.swift(1 hunks)Wable-iOS/Presentation/Overview/News/ViewModel/NewsViewModel.swift(2 hunks)Wable-iOS/Presentation/Overview/Notice/ViewModel/NoticeViewModel.swift(2 hunks)Wable-iOS/Presentation/Overview/Rank/View/RankListViewController.swift(2 hunks)Wable-iOS/Presentation/Profile/AccountInfo/View/AccountInfoViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift(3 hunks)Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift(2 hunks)Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift(3 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/Subview/WithdrawalGuideView.swift(2 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Reason/View/WithdrawalReasonViewController.swift(1 hunks)Wable-iOS/Presentation/Viewit/Create/View/Subview/ViewitInputView.swift(2 hunks)Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift(1 hunks)Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift(3 hunks)Wable-iOS/Presentation/WableComponent/ViewController/NotFoundViewController.swift(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/Subview/WithdrawalGuideView.swift (1)
Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/Subview/WithdrawalGuideDescriptionView.swift (1)
configure(33-35)
Wable-iOS/Presentation/Overview/Notice/ViewModel/NoticeViewModel.swift (1)
Wable-iOS/Domain/UseCase/Overview/OverviewUseCase.swift (2)
fetchNotices(43-45)fetchNotices(90-94)
🔇 Additional comments (53)
Wable-iOS/Core/Literals/String/StringLiterals+NotFound.swift (1)
14-16: IntroduceNotFound.subtitlein centralized literals
Centralizing this “page not found” message underStringLiterals.NotFound.subtitleis a great improvement for maintainability and localization consistency.Wable-iOS/Presentation/WableComponent/ViewController/NotFoundViewController.swift (1)
29-29: Replace hardcoded subtitle with centralized literal
Good refactor—subtitleLabelnow usesStringLiterals.NotFound.subtitle.pretendardString(with: .head2)instead of a hardcoded string, keeping UI text consistent.Wable-iOS/Presentation/Overview/Detail/View/Subview/AnnouncementDetailView.swift (1)
79-79: Use centralized feedback button title
Excellent—submitButtonnow usesStringLiterals.Overview.feedbackButtonShortTitlefor its attributed title, replacing the hardcoded string. This aligns with the overall literals refactor.Wable-iOS/Core/Literals/String/StringLiterals.swift (1)
10-10: Create rootStringLiteralsnamespace
Adding an emptyenum StringLiterals {}provides a clear root namespace for all string literal extensions. This foundation will help organize and discover constants across the codebase.Wable-iOS/Presentation/Community/View/CommunityRegisterCompleteViewController.swift (1)
57-57: Excellent refactoring to centralized string literals!The replacement of the hardcoded Korean string with
StringLiterals.Community.completeSheetTitleimproves maintainability and supports better localization practices.Wable-iOS/Presentation/Viewit/Create/View/Subview/ViewitInputView.swift (1)
27-27: Good refactoring to centralized literals!The replacement with
StringLiterals.Viewit.urlPlaceholderimproves maintainability and localization support.Wable-iOS/Presentation/Overview/Detail/View/AnnouncementDetailViewController.swift (1)
119-119: Excellent centralization of URL management!Replacing the local constant with
StringLiterals.URL.feedbackFormimproves maintainability and provides a single source of truth for URL management across the application.Wable-iOS/Presentation/Profile/Withdrawal/Reason/View/WithdrawalReasonViewController.swift (1)
73-78:Details
✅ Verification successful
Great refactoring to centralized profile literals!
The replacement of hardcoded strings with
StringLiterals.Profile.withdrawalReasonTitleandStringLiterals.Profile.withdrawalReasonSubtitlefollows the established centralization pattern and improves maintainability.Let me verify that the new centralized literals exist and contain the expected values:
🏁 Script executed:
#!/bin/bash # Description: Verify that the StringLiterals.Profile constants exist and are properly defined # Expected: Find the Profile extension with the withdrawal reason strings ast-grep --pattern $'extension StringLiterals { $$$ enum Profile { $$$ withdrawalReasonTitle $$$ withdrawalReasonSubtitle $$$ } $$$ }'Length of output: 163
I couldn’t locate the
StringLiterals.Profiledefinitions via AST—let’s try a direct text search:
🏁 Script executed:
#!/bin/bash # Search for the centralized withdrawal reason literal definitions rg -n "withdrawalReasonTitle" . rg -n "withdrawalReasonSubtitle" .Length of output: 759
Centralized literals verified
ThewithdrawalReasonTitleandwithdrawalReasonSubtitleconstants are defined inWable-iOS/Core/Literals/String/StringLiterals+Profile.swiftwith the expected Korean text:
- Line 15:
static let withdrawalReasonTitle = "정말 떠나시는 건가요?"- Line 16:
static let withdrawalReasonSubtitle = "계정을 삭제하시려는 이유를 말해주세요\n서비스 개선에 중요한 자료로 활용하겠습니다"No further changes needed—these match the original hardcoded strings.
Wable-iOS/Presentation/Profile/AccountInfo/View/AccountInfoViewController.swift (1)
124-124: LGTM! Excellent centralization of URL constants.This change successfully replaces a hardcoded URL string with a centralized
StringLiterals.URL.termsreference, improving maintainability and consistency across the codebase.Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/Subview/WithdrawalGuideView.swift (1)
45-45: LGTM! Consistent string literal centralization.The refactoring successfully replaces hardcoded strings with centralized
StringLiterals.Profileconstants. All changes follow a consistent pattern and appropriately use the Profile namespace for withdrawal-related UI text.Also applies to: 50-50, 53-53, 70-70
Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift (1)
121-121: LGTM! Proper URL centralization.The refactoring appropriately replaces hardcoded URLs with centralized
StringLiterals.URLconstants for terms and privacy policy links. This improves maintainability and ensures consistent URL usage across the app.Wable-iOS/Core/Literals/String/StringLiterals+Viewit.swift (1)
10-18: LGTM! Well-organized string literal extension.The new
StringLiterals+Viewit.swiftfile follows the established pattern of organizing string literals by feature. The placeholder constants are appropriately named and will improve maintainability for Viewit-related UI text.Wable-iOS/App/SceneDelegate.swift (2)
174-174: LGTM! Consistent literal centralization for URLs and update messages.The refactoring properly centralizes iTunes URL, App Store URL, and update alert text using appropriate
StringLiterals.URLandStringLiterals.Updatenamespaces.Also applies to: 226-227, 231-231
230-230: Verify intentional button text change.The button text was changed from "지금 업데이트" to "업데이트 하기". This appears to be a user-facing text change beyond just moving to centralized literals. Please confirm this change is intentional and not an unintended modification during the refactoring.
Wable-iOS/Core/Literals/String/StringLiterals+Overview.swift (1)
10-18: Well-structured literal organization!The extension approach with nested enums provides excellent organization for grouping related string constants. The property names are descriptive and follow consistent naming conventions.
Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift (2)
435-435: Good centralization of URL constants.Replacing the local constant with
StringLiterals.URL.feedbackFormimproves maintainability and consistency across the app.
440-440: Consistent use of centralized string literals.Using
StringLiterals.Profile.logoutSheetTitlealigns with the centralization effort and improves maintainability.Wable-iOS/Presentation/Overview/Rank/View/RankListViewController.swift (2)
46-46: Consistent use of centralized string literals for UI text.The replacement of the local constant with
StringLiterals.Overview.feedbackButtonLongTitleimproves consistency and maintainability across the app.
117-117: Good centralization of URL constants.Using
StringLiterals.URL.feedbackFormmaintains consistency with other parts of the app that reference the same URL.Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift (1)
279-279: Consistent centralization of string literals.Replacing the local constant with
StringLiterals.Ban.sheetMessagealigns with the broader refactoring effort to centralize string literals throughout the app.Wable-iOS/Presentation/Home/ViewModel/HomeDetailViewModel.swift (1)
149-149:Details
✅ Verification successful
LGTM! Centralized literal usage improves maintainability.
The replacement of a local constant with
IntegerLiterals.initialCursoraligns well with the refactoring objective to centralize literal values.Please verify that
IntegerLiteralsis properly accessible in this file. I don't see an explicit import for the literals module - ensure it's available through existing imports or add the necessary import statement.
🏁 Script executed:
#!/bin/bash # Verify IntegerLiterals accessibility and usage patterns echo "Checking IntegerLiterals usage and imports..." # Find all files using IntegerLiterals to verify import patterns rg -A 2 -B 2 "IntegerLiterals\." --type swift # Check if IntegerLiterals.swift exists and its location fd "IntegerLiterals.swift"Length of output: 16205
🏁 Script executed:
#!/bin/bash echo "Imports in HomeDetailViewModel.swift (first 30 lines):" sed -n '1,30p' Wable-iOS/Presentation/Home/ViewModel/HomeDetailViewModel.swiftLength of output: 1318
IntegerLiterals is part of the same module – no import needed.
All uses ofIntegerLiterals.initialCursorcompile without an explicit import sinceCore/Literals/Integer/IntegerLiterals.swiftlives in the same target. No changes required.Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift (1)
262-262:Details
✅ Verification successful
LGTM! Centralized URL management improves maintainability.
Replacing the hardcoded Google Forms URL with
StringLiterals.URL.feedbackFormis a good practice that centralizes URL management and makes it easier to update URLs across the app.Please verify that
StringLiteralsis properly accessible in this file. Ensure the necessary import statement is present or that it's available through existing imports.
🏁 Script executed:
#!/bin/bash # Verify StringLiterals accessibility and URL consistency echo "Checking StringLiterals usage and feedbackForm URL..." # Find all files using StringLiterals.URL.feedbackForm rg "StringLiterals\.URL\.feedbackForm" --type swift # Check StringLiterals definition and URL values fd "StringLiterals" --type f --extension swift -x cat {}Length of output: 5102
StringLiterals.URL.feedbackForm Verified Accessible
I’ve confirmed that
StringLiterals.URL.feedbackFormis defined in StringLiterals+URL.swift and is reachable from all view controllers without additional imports. No further changes needed.Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift (1)
60-60: LGTM! Consistent pagination initialization.Using
IntegerLiterals.initialCursorstandardizes the initial cursor value across all pagination implementations.Wable-iOS/Presentation/Overview/News/ViewModel/NewsViewModel.swift (2)
47-47: LGTM! Consistent pagination initialization.Using
IntegerLiterals.initialCursormaintains consistency with other view models in the pagination refactoring.
110-110: LGTM! Centralized page size management.The replacement with
IntegerLiterals.defaultCountPerPagealigns well with the centralization effort and ensures consistent pagination behavior across all view models.Wable-iOS/Presentation/Viewit/List/ViewModel/ViewitListViewModel.swift (3)
163-163: Consistent usage of centralized literals.Good to see consistent usage of
IntegerLiterals.initialCursoracross different code paths (both initial load and ban action refresh).
242-242: Proper pagination logic with centralized constant.The replacement maintains the same logic while using the centralized
IntegerLiterals.defaultCountPerPageconstant.
73-73:Details
✅ Verification successful
Good refactoring to centralized literals.
The replacement of local
Constant.initialCursorwithIntegerLiterals.initialCursorimproves maintainability by centralizing pagination constants.Please verify that the
IntegerLiteralsenum is properly defined and contains the expected constants:
🏁 Script executed:
#!/bin/bash # Description: Verify IntegerLiterals enum exists and contains the expected constants # Search for IntegerLiterals definition ast-grep --pattern $'enum IntegerLiterals { $$$ }' # Also check for the specific constants being used rg -A 5 "IntegerLiterals" | rg -E "(initialCursor|defaultCountPerPage)"Length of output: 522
🏁 Script executed:
#!/bin/bash # Search for the IntegerLiterals enum definition in Swift files rg -n "enum IntegerLiterals" --glob "*.swift" # Search for the constants within that enum rg -n "static let initialCursor" --glob "*.swift" rg -n "static let defaultCountPerPage" --glob "*.swift" # As a fallback, locate any occurrences of these identifiers rg -n "\b(initialCursor|defaultCountPerPage)\b" --glob "*.swift"Length of output: 4079
IntegerLiterals constants verified
I’ve located the
IntegerLiteralsenum inWable-iOS/Core/Literals/Integer/IntegerLiterals.swiftand confirmed it defines:
static let initialCursor = -1(line 11)static let defaultCountPerPage = 15(line 13)Refactoring ViewitListViewModel.swift (and related files) to use these centralized literals is safe and improves maintainability. Approving these changes.
Wable-iOS/Presentation/Home/ViewModel/HomeViewModel.swift (2)
112-112: Appropriate use of centralized initial cursor.The replacement with
IntegerLiterals.initialCursoris correct and consistent with the refactoring pattern.
118-118: Good use of home-specific pagination constant.Using
IntegerLiterals.homeContentCountPerPageinstead of a generic constant is a good practice, as it allows home screen to have different pagination settings than other screens. This aligns with the PR objective to correct home pagination from 10 to 20 items.Also applies to: 142-142
Wable-iOS/Presentation/Notification/Information/ViewModel/InformationNotificationViewModel.swift (2)
49-49: Consistent centralization of pagination constants.Good replacement of local
Constant.initialCursorwith the centralizedIntegerLiterals.initialCursor.
105-105: Proper use of default pagination constant.The replacement with
IntegerLiterals.defaultCountPerPageis appropriate for notification screens, using the standard pagination size rather than a screen-specific one.Wable-iOS/Presentation/Overview/Notice/ViewModel/NoticeViewModel.swift (2)
47-47: Consistent refactoring pattern.Good replacement of local constant with centralized
IntegerLiterals.initialCursor, maintaining the same functionality while improving maintainability.
108-108: Appropriate use of default pagination constant.Using
IntegerLiterals.defaultCountPerPageis correct for notice screens, following the standard pagination approach.Wable-iOS/Presentation/Community/View/CommunityViewController.swift (2)
246-248: Good use of centralizedStringLiterals.Communityconstants
Replacing hardcoded sheet titles and messages with centralized literals improves consistency and prepares for localization.
256-258: Appropriate invocation of centralized URL literals
UsingStringLiterals.URL.littlyhere ensures consistent URL management and avoids scattered hardcoded links.Wable-iOS/Presentation/Profile/Other/ViewModel/OtherProfileViewModel.swift (4)
233-236: Consistent use ofIntegerLiteralsfor initial cursors
Switching toIntegerLiterals.initialCursorcentralizes pagination parameters, improving maintainability and reducing magic numbers in your code.Also applies to: 238-241
245-249: Verify pagination boundary logic
Using< IntegerLiterals.defaultCountPerPageto setisLastPageForContentis correct, but ensure the server-side default page size matchesdefaultCountPerPage; otherwise,isLastPageForContentmay be miscalculated.
274-281: Pagination check after loading more items
Good application ofIntegerLiterals.defaultCountPerPagewhen updatingisLastPageForContent, which prevents unnecessary further fetches once the last page is reached.
299-306: Pagination check for comment list
Correct use ofIntegerLiterals.profileCommentCountPerPageensures consistent page-size logic for comments across initial and subsequent fetches.Wable-iOS/Presentation/Profile/My/ViewModel/MyProfileViewModel.swift (4)
163-167: Centralize pagination cursors usingIntegerLiterals.initialCursor
Replacing magic numbers withIntegerLiterals.initialCursorin both content and comment fetches improves consistency across view models.Also applies to: 169-172
175-180: Confirm alignment between backend andIntegerLiteralspage sizes
Ensure the server-side default page size matchesIntegerLiterals.defaultCountPerPageandprofileCommentCountPerPageto prevent off-by-one issues inisLastPageFor...logic.
211-214: Maintain consistency in load-more logic
UpdatingisLastPageForContentwithIntegerLiterals.defaultCountPerPageafter each fetch ensures the load-more boundary logic aligns with the initial pagination settings.
235-238: Maintain consistency in comment pagination logic
UsingIntegerLiterals.profileCommentCountPerPagehere matches the initial fetch logic and ensures a unified page size for comments.Wable-iOS/Core/Literals/String/StringLiterals+Community.swift (1)
14-28: Great centralization of community-related string literals
Having a dedicatedStringLiterals.Communityenum for all community UI text significantly improves maintainability and localization readiness.Wable-iOS/Core/Literals/String/StringLiterals+Profile.swift (1)
10-27: Well-structured centralized string literalsThe new nested
Profileenum cleanly organizes profile-related UI strings and aligns with the existingStringLiteralspattern, improving maintainability and preparing the code for future localization efforts.Wable-iOS.xcodeproj/project.pbxproj (7)
296-305: New literal sources added to build phase
These entries correctly includeIntegerLiterals.swiftand allStringLiteralsvariants in the PBXBuildFile section, ensuring they’re compiled.
643-652: Register literal files in file reference section
The PBXFileReference entries for the new literal files appear correct and match the added build file identifiers.
820-820: Insert “Literals” group under Core
The newLiteralsgroup is properly nested within theCoregroup, organizing literal enums in one place.
2001-2009: Define rootLiteralsPBXGroup
TheLiteralsgroup is defined with two child groups (StringandInteger), matching the intended project structure.
2010-2017: DefineIntegersubgroup
TheIntegersubgroup correctly containsIntegerLiterals.swiftas its sole child.
2018-2033: DefineStringsubgroup
TheStringsubgroup lists allStringLiteralsextensions, centralizing string constants.
2611-2611: Add literal files to the Sources build phase
The new literal files are now referenced in the PBXSourcesBuildPhase section, ensuring they’re included in compilation. Verify there are no duplicates and that ordering is consistent with other source entries.Also applies to: 2625-2625, 2634-2634, 2636-2636, 2658-2658, 2715-2715, 2823-2823, 2878-2878
|
|
||
| @objc func nextButtonDidTap() { | ||
| let wableSheet = WableSheetViewController(title: "계정을 삭제하시겠어요?") | ||
| let wableSheet = WableSheetViewController(title: StringLiterals.Profile.withdrawalSheetTitle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Centralize sheet action titles
The sheet’s title is now correctly pulled from StringLiterals.Profile.withdrawalSheetTitle, but the cancel and delete button titles remain hardcoded. To keep all UI text centralized, consider adding withdrawalCancelTitle and withdrawalConfirmTitle (or similar) to StringLiterals.Profile and using them here.
--- a/Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift
+++ b/Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift
@@ -141,7 +141,7 @@
- let cancelAction = WableSheetAction(title: "취소", style: .gray)
+ let cancelAction = WableSheetAction(title: StringLiterals.Profile.withdrawalCancelTitle, style: .gray)
let withdrawAction = WableSheetAction(title: "삭제하기", style: .primary) { [weak self] in
- self?.withdrawRelay.send()
+ self?.withdrawRelay.send()
}
wableSheet.addActions(cancelAction, withdrawAction)
present(wableSheet, animated: true)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift
at line 142, the sheet's cancel and delete button titles are hardcoded while the
sheet title uses StringLiterals.Profile.withdrawalSheetTitle. To centralize all
UI text, add new string constants like withdrawalCancelTitle and
withdrawalConfirmTitle to StringLiterals.Profile and replace the hardcoded
button titles with these constants in the WableSheetViewController
initialization or configuration.
| enum Constant { | ||
| static let defaultItemsCountPerPage: Int = 15 | ||
| static let initialCursor: Int = -1 | ||
| return notifications.isEmpty || notifications.count < IntegerLiterals.defaultCountPerPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify pagination page size correction.
The change to IntegerLiterals.defaultCountPerPage looks good for centralization. However, the PR objectives mention a pagination correction from 10 to 20. Please verify that this constant reflects the correct page size.
🏁 Script executed:
#!/bin/bash
# Verify the pagination page size values in IntegerLiterals
echo "Checking IntegerLiterals pagination values..."
# Find IntegerLiterals definition and check defaultCountPerPage value
ast-grep --pattern 'enum IntegerLiterals {
$$$
}'
# Also check for any remaining hardcoded pagination values that might need updating
rg "(?:count.*per.*page|page.*size|items.*per.*page)" --type swift -i
Length of output: 3158
Update defaultCountPerPage to 20 to match pagination objectives
The grep results show that defaultCountPerPage is still set to 15, but the PR goals call for increasing page size from 10 to 20. Please update the constant and ensure all uses of defaultCountPerPage now resolve to 20.
• Core/Literals/Integer/IntegerLiterals.swift
- static let defaultCountPerPage = 15
+ static let defaultCountPerPage = 20
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return notifications.isEmpty || notifications.count < IntegerLiterals.defaultCountPerPage | |
| // Core/Literals/Integer/IntegerLiterals.swift | |
| enum IntegerLiterals { | |
| // ... other literals ... | |
| static let defaultCountPerPage = 20 | |
| // ... other literals ... | |
| } |
🤖 Prompt for AI Agents
In
Wable-iOS/Presentation/Notification/Activity/ViewModel/ActivityNotificationViewModel.swift
at line 184, the code uses IntegerLiterals.defaultCountPerPage for pagination
size, but the constant is currently set to 15 instead of the required 20 as per
the PR objectives. To fix this, update the defaultCountPerPage value to 20 in
Core/Literals/Integer/IntegerLiterals.swift and verify all references to this
constant reflect the new value to ensure consistent pagination behavior.
…xit, Empty, Detail, Delete Literal 작성
…Exit, Empty, Detail, Delete Literal 적용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
Wable-iOS/Presentation/Home/View/HomeViewController.swift (2)
299-313: 🛠️ Refactor suggestionAdditional hardcoded strings need refactoring.
The action button titles ("고민할게요", "네 맞아요") are still hardcoded and should follow the same StringLiterals pattern used elsewhere in this PR.
200-264: 💡 Verification agent🧩 Analysis chain
Inconsistent application of StringLiterals refactoring.
While some strings have been successfully moved to StringLiterals, many hardcoded Korean strings remain throughout this section (e.g., "삭제하기", "취소", "신고하기", "밴하기", "신고하시겠어요?"). This creates inconsistency in the codebase.
Consider completing the refactoring by moving all remaining hardcoded strings to appropriate StringLiterals namespaces for consistency and maintainability.
🏁 Script executed:
#!/bin/bash # Description: Find all hardcoded Korean strings in this file to assess refactoring completeness # Expected: List of remaining hardcoded Korean text that should potentially be moved to StringLiterals rg -n '[가-힣]+' --type swift Wable-iOS/Presentation/Home/View/HomeViewController.swiftLength of output: 1105
Complete StringLiterals refactoring in HomeViewController.swift
Multiple hardcoded Korean strings remain in Wable-iOS/Presentation/Home/View/HomeViewController.swift (lines 200–306). Extract all of them into the appropriate StringLiterals cases for consistency and maintainability:
• Line 200: title: "삭제하기"
• Lines 205, 226, 250: title: "취소"
• Line 207: title: "삭제하기"
• Lines 221, 228, 245, 252: title: "신고하기"
• Line 240: title: "밴하기"
• Line 247: title: "신고하시겠어요?"
• Line 299: title: "고민할게요"
• Line 306: title: "네 맞아요"Please consolidate these into StringLiterals to align with the existing refactoring.
♻️ Duplicate comments (2)
Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift (2)
129-129: Consistent refactoring, but same typo concern.Good centralization of string literals, but this references the same potentially misspelled properties (
checkVaildMessage) mentioned in the previous file review.
215-215: Same typo concern applies here.References the same potentially misspelled
checkInvaildErrorproperty.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
Wable-iOS/Core/Literals/String/StringLiterals+Empty.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Onboarding.swift(1 hunks)Wable-iOS/Core/Literals/String/StringLiterals+Report.swift(1 hunks)Wable-iOS/Presentation/Home/View/HomeDetailViewController.swift(10 hunks)Wable-iOS/Presentation/Home/View/HomeViewController.swift(5 hunks)Wable-iOS/Presentation/Home/View/WritePostViewController.swift(6 hunks)Wable-iOS/Presentation/Login/LoginViewController.swift(2 hunks)Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift(2 hunks)Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift(1 hunks)Wable-iOS/Presentation/Onboarding/View/AgreementView.swift(2 hunks)Wable-iOS/Presentation/Onboarding/View/LCKTeamView.swift(2 hunks)Wable-iOS/Presentation/Onboarding/View/LCKYearView.swift(1 hunks)Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift(4 hunks)Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift(2 hunks)Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift(3 hunks)Wable-iOS/Presentation/Overview/News/View/NewsViewController.swift(1 hunks)Wable-iOS/Presentation/Overview/Notice/View/NoticeViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift(3 hunks)Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift(1 hunks)Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift(3 hunks)Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift(7 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/Subview/WithdrawalGuideView.swift(2 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift(1 hunks)Wable-iOS/Presentation/Profile/Withdrawal/Reason/View/WithdrawalReasonViewController.swift(1 hunks)Wable-iOS/Presentation/Viewit/Create/View/CreateViewitViewController.swift(1 hunks)Wable-iOS/Presentation/Viewit/List/View/Subview/ViewitListView.swift(1 hunks)Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift(2 hunks)
✅ Files skipped from review due to trivial changes (12)
- Wable-iOS/Presentation/Onboarding/View/LCKYearView.swift
- Wable-iOS/Presentation/Overview/Notice/View/NoticeViewController.swift
- Wable-iOS/Presentation/Notification/Information/View/InformationNotificationViewController.swift
- Wable-iOS/Presentation/Viewit/List/View/Subview/ViewitListView.swift
- Wable-iOS/Presentation/Viewit/Create/View/CreateViewitViewController.swift
- Wable-iOS/Presentation/Overview/News/View/NewsViewController.swift
- Wable-iOS/Presentation/Onboarding/View/AgreementView.swift
- Wable-iOS/Presentation/Login/LoginViewController.swift
- Wable-iOS/Presentation/Onboarding/View/LCKTeamView.swift
- Wable-iOS/Presentation/Onboarding/View/ProfileRegisterView.swift
- Wable-iOS/Presentation/Profile/My/View/Cell/MyProfileEmptyCell.swift
- Wable-iOS/Presentation/Profile/My/View/MyProfileViewController.swift
🚧 Files skipped from review as they are similar to previous changes (9)
- Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/WithdrawalGuideViewController.swift
- Wable-iOS/Presentation/Notification/Activity/View/ActivityNotificationViewController.swift
- Wable-iOS/Presentation/Profile/Withdrawal/Reason/View/WithdrawalReasonViewController.swift
- Wable-iOS/Presentation/Onboarding/ViewController/AgreementViewController.swift
- Wable-iOS/Presentation/Profile/Withdrawal/Guide/View/Subview/WithdrawalGuideView.swift
- Wable-iOS/Core/Literals/String/StringLiterals+Report.swift
- Wable-iOS/Presentation/Profile/Other/View/OtherProfileViewController.swift
- Wable-iOS/Presentation/Viewit/List/View/ViewitListViewController.swift
- Wable-iOS/Core/Literals/String/StringLiterals+Onboarding.swift
🔇 Additional comments (20)
Wable-iOS/Presentation/Home/View/HomeDetailViewController.swift (5)
219-219: Excellent use of centralized string literals for deletion confirmation.The replacement of hardcoded strings with
StringLiterals.Delete.contentSheetTitleandStringLiterals.Delete.contentSheetMessageimproves maintainability and supports future localization efforts.
240-240: Consistent application of centralized reporting strings.Good consistency in replacing all hardcoded report sheet titles with
StringLiterals.Report.sheetTitleacross multiple reporting scenarios (admin and regular user flows).Also applies to: 264-264, 371-371, 401-401
290-290: Proper centralization of ghost feature strings.The replacement with
StringLiterals.Ghost.sheetTitlemaintains consistency in the ghost reporting feature across both content and comment contexts.Also applies to: 452-452
344-344: Well-organized comment deletion confirmation strings.The use of separate
StringLiterals.Delete.commentSheetTitleandStringLiterals.Delete.commentSheetMessageshows good separation of concerns between content and comment deletion flows.
635-635: Appropriate categorization of success and completion messages.The toast messages are properly categorized under
StringLiterals.Detail.rippleCompleteToastandStringLiterals.Report.completeToast, maintaining logical grouping by feature area.Also applies to: 648-648
Wable-iOS/Presentation/Home/View/HomeViewController.swift (5)
62-62: Good refactoring to use centralized string literals.The replacement of hardcoded text with
StringLiterals.Empty.postimproves maintainability and supports internationalization.
202-202: Excellent use of centralized string constants.Both
StringLiterals.Delete.contentSheetTitleandStringLiterals.Delete.contentSheetMessageproperly replace hardcoded strings for the deletion confirmation dialog.
223-223: Consistent application of StringLiterals pattern.The use of
StringLiterals.Report.sheetTitlemaintains consistency with the refactoring approach.
295-295: Well-organized string literal usage.The
StringLiterals.Ghost.sheetTitlefollows the established naming convention and grouping pattern.
455-455: Good completion of toast message refactoring.The
StringLiterals.Report.completeToastproperly centralizes the success message string.Wable-iOS/Core/Literals/String/StringLiterals+Empty.swift (1)
11-24: Well-structured centralized string literals implementation.The
Emptyenum provides a clean, organized way to manage empty state messages. The Korean localization strings cover all necessary empty state scenarios, and the static property approach follows Swift best practices for string constants.Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (2)
195-195: LGTM!Good use of centralized string literals for the photo permission alert message. This improves consistency across the app.
263-263: Same typo concern as line 137.The same potential typo issue with "checkInvaildError" - should likely be "checkInvalidError".
Wable-iOS/Presentation/Onboarding/ViewController/ProfileRegisterViewController.swift (1)
170-170: LGTM!Consistent use of centralized photo permission message.
Wable-iOS/Presentation/Home/View/WritePostViewController.swift (6)
34-34: Excellent refactoring of placeholder text initialization.Clean replacement of hardcoded placeholder strings with centralized constants. This improves maintainability and localization support.
Also applies to: 42-42
228-228: Properly updated string comparisons to use centralized literals.Good job maintaining the logic integrity while updating string comparisons to use the new StringLiterals constants. The null-checking and placeholder detection logic remains correct.
Also applies to: 235-237
240-240: LGTM!Appropriate use of centralized exit sheet title constant.
274-274: Character validation logic correctly updated.The post button enablement logic properly uses the new string literal for placeholder comparison while maintaining the original validation behavior.
295-295: Consistent photo permission handling.Good use of the centralized photo permission message, maintaining consistency across the app.
331-331: Well-refactored text view delegate methods.The placeholder handling in
textViewDidBeginEditingandtextViewDidEndEditingcorrectly uses the centralized string literals while preserving the original behavior.Also applies to: 340-340
| let condition = completion == .finished | ||
|
|
||
| self?.rootView.conditiionLabel.text = condition ? "사용 가능한 닉네임입니다." : "이미 사용 중인 닉네임입니다." | ||
| self?.rootView.conditiionLabel.text = condition ? StringLiterals.ProfileSetting.checkVaildMessage : StringLiterals.ProfileSetting.checkDuplicateError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Flag potential typos in StringLiterals property names.
The referenced constants checkVaildMessage and checkDuplicateError appear to have a typo - "Vaild" should likely be "Valid". Please verify if StringLiterals.ProfileSetting.checkValidMessage was intended instead.
🏁 Script executed:
#!/bin/bash
# Description: Check for potential typos in StringLiterals.ProfileSetting property names
# Expected: Find the actual property names to confirm if there are typos
ast-grep --pattern 'enum ProfileSetting {
$$$
}'
# Also search for any usage of these properties to see the pattern
rg -A 2 -B 2 'checkVaildMessage|checkInvaildError'
Length of output: 4867
Correct typos in ProfileSetting string literal names
Two constants in StringLiterals+ProfileSetting.swift are misspelled (“Vaild” and “Invaild”) and should be renamed along with all their usages:
• In Wable-iOS/Core/Literals/String/StringLiterals+ProfileSetting.swift:
- static let checkVaildMessage = "사용 가능한 닉네임입니다."
+ static let checkValidMessage = "사용 가능한 닉네임입니다."
- static let checkInvaildError = "닉네임에 사용할 수 없는 문자가 포함되어 있어요."
+ static let checkInvalidError = "닉네임에 사용할 수 없는 문자가 포함되어 있어요."
• In usages (update in both Edit and Register view controllers):
- StringLiterals.ProfileSetting.checkVaildMessage
+ StringLiterals.ProfileSetting.checkValidMessage
- StringLiterals.ProfileSetting.checkInvaildError
+ StringLiterals.ProfileSetting.checkInvalidError
Please rename the properties and update all references to maintain consistency and avoid confusion.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self?.rootView.conditiionLabel.text = condition ? StringLiterals.ProfileSetting.checkVaildMessage : StringLiterals.ProfileSetting.checkDuplicateError | |
| self?.rootView.conditiionLabel.text = condition | |
| ? StringLiterals.ProfileSetting.checkValidMessage | |
| : StringLiterals.ProfileSetting.checkDuplicateError |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift at line
137, the string literal property names checkVaildMessage and checkDuplicateError
contain typos; specifically, "Vaild" should be corrected to "Valid". To fix
this, rename the properties in the StringLiterals+ProfileSetting.swift file from
checkVaildMessage to checkValidMessage and from checkInvaildError to
checkInvalidError, then update all references to these properties throughout the
codebase, including this file and any other view controllers like the Register
view controller, to maintain consistency and prevent errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿 빠른 테플 반영을 위해 머지하겠습니다 ~
👻 PULL REQUEST
📄 작업 내용
진웅IntegerLiterals를 구현하였습니다.진웅StringLiterals는 맡은 부분 구현하였습니다.유진Login, Onboarding, Profile, Write, Report, Ghost, Exit, Empty, Detail, Delete에 대한 StringLiterals를 구현했어요.유진구현된 StringLiterals에 맞춰 프로젝트에 적용했어요.📚 참고자료
진웅기존에 토의하지 않았던, 업데이트를 위한 링크들도 StringLiterals에 포함하였습니다.진웅제가 맡은 부분은 모두 구현하였기에, 폴더 분류는 삭제하였습니다.유진해당 부분 충돌나서 해결했는데 해결 과정에서pbxproj를 좀 건드려서 한번 풀 받으시면 좋을 것 같습니다!진웅홈 화면 기준 페이지네이션 기준 숫자는 10이 아닌 20이었기에 이를 반영하여 적용하였습니다.진웅프로퍼티 네이밍에 대한 리뷰는 언제나 환영입니다~👀 기타 더 이야기해볼 점
진웅현재 브랜치에서 바로 작업을 이어나가셔도 되고, 리뷰를 남기고 머지 후 작업을 이어나가셔도 괜찮을 것 같습니다.진웅상세페이지에서 페이지네이션에 사용되어야 할 조건이 누락된 것 같습니다.IntegerLiterals.defaultCountPerPage를 이용한 비교 검사가 필요해 보입니다.유진말씀해주신 부분 구현했습니다! 다만 상세페이지에서 댓글 페이지네이션 시 사용하는 조건이 댓글 기준(답글 포함 x) 10개로 설정되어 있어 기존IntegerLiterals.profileCommentCountPerPage를IntegerLiterals.commentCountPerPage로 리팩토링 후 사용했습니다.유진진웅님께서 작성하신Profileenum과 제가 작성한Profileenum이 충돌해 두 enum의 네이밍을 각각ProfileDelete(진웅),ProfileSetting(유진)으로 설정해두었습니다.🔗 연결된 이슈
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Style