I've improved the KDocs and inline comments in DateTimeDiffer and `…
#413
Conversation
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
…DateTimeDifferTest`.
This will enhance the documentation and comments in `DateTimeDiffer.kt` and its corresponding test file `DateTimeDifferTest.kt`.
Here are the key changes:
- **`DateTimeDiffer.kt`**:
- I updated the KDocs for `diffWorkingHours` and `workingDuration` to include explicit `@param`, `@return`, and `@throws` tags.
- I restructured KDoc descriptions for better clarity on function behavior, assumptions, and handling of edge cases (e.g., times outside working hours, weekends).
- I removed commented-out debugging code.
- I converted a "Future Note" to a `TODO` comment.
- I improved inline comments to explain the "why" behind complex logic, not just the "what".
- I replaced informal comments with professional explanations.
- **`DateTimeDifferTest.kt`**:
- I reviewed and confirmed the accuracy of the main class KDoc.
- I removed an overly verbose multi-line comment block, replacing it with a concise explanation.
- I corrected a misleading comment in one of the test cases.
- I ensured test method names are descriptive and inline comments are helpful and accurate.
These changes aim to make your code easier to understand, maintain, and use.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances documentation and inline comments in DateTimeDiffer and its tests for clarity and maintainability.
- Refactored KDoc in
DateTimeDiffer.kt: added explicit tags, restructured descriptions, removed debug code, and added a TODO. - Cleaned up verbose comments in
DateTimeDifferTest.kt: replaced with concise, scenario-based inline comments. - Improved inline explanations to focus on the “why” of complex logic, not just the “what.”
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main/kotlin/dev/hossain/time/DateTimeDiffer.kt | Enhanced KDoc, removed debugging blocks, added TODO, clarified logic with inline comments. |
| src/test/kotlin/dev/hossain/time/DateTimeDifferTest.kt | Simplified comments, added realistic test scenario notes, removed large commented-out sections. |
Comments suppressed due to low confidence (2)
src/test/kotlin/dev/hossain/time/DateTimeDifferTest.kt:130
- [nitpick] The test name uses
diffinstead of the method under test,diffWorkingHours. Rename the test to start withdiffWorkingHours - given…to maintain consistency with other tests.
fun `diff - given start time is before working hour and end time during work hour - provides diff of working hour only`() {
src/main/kotlin/dev/hossain/time/DateTimeDiffer.kt:109
- This assignment overrides the
previousWorkingDaycalculated just above fromnextWorkingDayAfterWeekend, potentially skipping the intended start time. Consider reusingnextWorkingDayAfterWeekenddirectly rather than chaining off the originalpreviousWorkingDay.
previousWorkingDay = previousWorkingDay.nextWorkingDay().prevWorkingHour()
| * | ||
| * @param startInstant The start instant of the period. | ||
| * @param endInstant The end instant of the period. | ||
| * @param timeZoneId The ZoneId to determine local working days and hours. |
There was a problem hiding this comment.
The KDoc parameter name timeZoneId does not match the function signature which uses zoneId. Update the @param to zoneId for accuracy.
Copilot uses AI. Check for mistakes.
| * ↑ ↑ | ||
| * Start End | ||
| * of Day of Day | ||
| * ↑ ↑ ↑ |
There was a problem hiding this comment.
[nitpick] The ASCII art arrows and labels for week start, week end, and weekend are misaligned and can be confusing. Consider simplifying or realigning this diagram for clarity, or removing it if it adds more complexity than benefit.
Copilot uses AI. Check for mistakes.
|
Has some good inline comments for different cases. Salvage them later. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…DateTimeDifferTest`.
This will enhance the documentation and comments in
DateTimeDiffer.ktand its corresponding test fileDateTimeDifferTest.kt.Here are the key changes:
DateTimeDiffer.kt:diffWorkingHoursandworkingDurationto include explicit@param,@return, and@throwstags.TODOcomment.DateTimeDifferTest.kt:These changes aim to make your code easier to understand, maintain, and use.