Rename noChildren to hasChildren for clarity#599
Rename noChildren to hasChildren for clarity#599gioalex07 wants to merge 5 commits intoVREMSoftwareDevelopment:mainfrom
Conversation
The property noChildren returned true when children were present, which was semantically misleading. Renamed to hasChildren to match the actual behavior and avoid confusion for future contributors.
|
Thanks for the contribution.
Please update the PR with real‑device testing details so review can continue. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #599 +/- ##
=========================================
Coverage 97.83% 97.83%
Complexity 975 975
=========================================
Files 121 121
Lines 2581 2581
Branches 211 211
=========================================
Hits 2525 2525
Misses 19 19
Partials 37 37 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Renames a misleading WiFiDetail boolean property so its name matches its behavior, and updates affected production/test call sites accordingly.
Changes:
- Renamed
WiFiDetail.noChildren→WiFiDetail.hasChildren. - Updated
AccessPointsAdapterGroupto use the new property name. - Updated unit tests to reference
hasChildren(assertions and some test names).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/src/main/kotlin/com/vrem/wifianalyzer/wifi/model/WiFiDetail.kt | Renames the boolean property to hasChildren while keeping the same underlying logic. |
| app/src/main/kotlin/com/vrem/wifianalyzer/wifi/accesspoint/AccessPointsAdapterGroup.kt | Updates the two call sites to use hasChildren. |
| app/src/test/kotlin/com/vrem/wifianalyzer/wifi/model/WiFiDetailTest.kt | Updates test names and assertions to validate hasChildren. |
| app/src/test/kotlin/com/vrem/wifianalyzer/wifi/accesspoint/AccessPointsAdapterGroupTest.kt | Updates mocking/verification to use hasChildren (but leaves some test names semantically inconsistent with the stubbed values). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fun onGroupCollapsedDoesNotRemoveIfHasChildren() { | ||
| // setup | ||
| doReturn(GroupBy.SSID).whenever(settings).groupBy() | ||
| fixture.updateGroupBy() | ||
| doReturn(false).whenever(wiFiDetailWithChildren).noChildren | ||
| doReturn(false).whenever(wiFiDetailWithChildren).hasChildren | ||
| val wiFiDetails = listOf(wiFiDetailWithChildren) | ||
| fixture.expanded.add("test") | ||
| // execute | ||
| fixture.onGroupCollapsed(wiFiDetails, 0) | ||
| // validate | ||
| assertThat(fixture.expanded).contains("test") | ||
| verify(wiFiDetailWithChildren).noChildren | ||
| verify(wiFiDetailWithChildren).hasChildren |
There was a problem hiding this comment.
The test name says "DoesNotRemoveIfHasChildren", but this test stubs hasChildren to false and asserts the expanded set is unchanged. Rename the test to match the behavior being exercised (e.g., "...IfNoChildren"), or change the stub/assertions to actually cover the hasChildren == true path implied by the current name.
| @Test | ||
| fun onGroupExpandedDoesNotAddIfHasChildren() { | ||
| // setup | ||
| doReturn(GroupBy.SSID).whenever(settings).groupBy() | ||
| fixture.updateGroupBy() | ||
| doReturn(false).whenever(wiFiDetailWithChildren).noChildren | ||
| doReturn(false).whenever(wiFiDetailWithChildren).hasChildren | ||
| val wiFiDetails = listOf(wiFiDetailWithChildren) | ||
| // execute | ||
| fixture.onGroupExpanded(wiFiDetails, 0) | ||
| // validate | ||
| assertThat(fixture.expanded).isEmpty() | ||
| verify(wiFiDetailWithChildren).noChildren | ||
| verify(wiFiDetailWithChildren).hasChildren |
There was a problem hiding this comment.
The test name says "DoesNotAddIfHasChildren", but it stubs hasChildren to false and expects no change. Update the test name to reflect the hasChildren == false scenario, or adjust the setup/expectation so the name and behavior align.
The property noChildren returned true when children were present, which was semantically misleading. Renamed to hasChildren to match the actual behavior and avoid confusion for future contributors.
Summary
WiFiDetail.noChildrentohasChildrenfor semantic clarity.noChildrenreturnedtruewhen children were present.What does this implement/fix?
WiFiDetail.ktAccessPointsAdapterGroup.kt(2 call sites)WiFiDetailTest.ktandAccessPointsAdapterGroupTest.ktDoes this close any issues?
How was this tested?