Skip to content

Commit e8bcc8e

Browse files
bedaHovorkaclaude
bedaHovorka
and
claude
committed
Address PR #357 inline review feedback
- Rename editingContextFactory to xmlContextFactory in OpenAction to avoid confusion with EditingContextFactory (reviewer comment MenuBar.kt:73) - Fix placeholder comment Issue #XXX PR #358 to Issue #80 (MenuBar.kt:103) - Rename inOutCount to inOutsCount for consistency (MenuBar.kt:202) - Rename docs/issues/issue-80.md to issue_80.md to match underscore convention - Update issue_80.md dialog text excerpt to match current implementation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 39cfec0 commit e8bcc8e

4 files changed

Lines changed: 78 additions & 73 deletions

File tree

‎desktop-ui/src/main/kotlin/cz/vutbr/fit/interlockSim/gui/MenuBar.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ class MenuBar : JMenuBar() {
6969

7070
try {
7171
// Use lenient parsing to separate unparseable XML from validation errors
72-
val editingContextFactory = getKoin().get<XMLContextFactory>()
73-
val parseResult = editingContextFactory.createContextLenient(selectedFile)
72+
val xmlContextFactory = getKoin().get<XMLContextFactory>()
73+
val parseResult = xmlContextFactory.createContextLenient(selectedFile)
7474

7575
when {
7676
// Case 1: Successfully parsed with no errors
@@ -100,7 +100,7 @@ class MenuBar : JMenuBar() {
100100
frame.modificationTracker.setCurrentFile(selectedFile)
101101
frame.modificationTracker.markClean()
102102

103-
// Check if context has insufficient InOuts and show warning (Issue #XXX, PR #358)
103+
// Check if context has insufficient InOuts and show warning (Issue #80)
104104
val inOutsCount = context.getInOuts().size
105105
val minRequired = XMLContextFactory.MIN_INOUT_ELEMENTS
106106
if (inOutsCount < minRequired) {
@@ -193,13 +193,13 @@ class MenuBar : JMenuBar() {
193193
val editingContext = frame.railwayNetGridCanvas.getEditingContext()
194194

195195
// Validate InOut count before saving (Issue #80)
196-
val inOutCount = editingContext.getInOuts().size
196+
val inOutsCount = editingContext.getInOuts().size
197197
val minRequired = XMLContextFactory.MIN_INOUT_ELEMENTS
198198
if (!validateForSave(editingContext)) {
199199
JOptionPane.showMessageDialog(
200200
this,
201201
"Railway network must have at least $minRequired InOut elements (entry and exit points).\n\n" +
202-
"Current count: $inOutCount\n\n" +
202+
"Current count: $inOutsCount\n\n" +
203203
"InOut elements define where trains enter and exit the railway network.\n" +
204204
"Please add more InOut elements before saving.",
205205
"Cannot Save - Insufficient InOut Elements",

‎docs/issues/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ This directory contains detailed documentation for resolved issues in the Railwa
66

77
**Status:** ✅ Implemented (2026-02-06)
88

9-
**File:** [`issue-80.md`](issue-80.md)
9+
**File:** [`issue_80.md`](issue_80.md)
1010

1111
**Summary:**
1212
Implemented GUI validation to prevent saving railway networks with 0 InOut elements. Pre-save check in `MenuBar.performSave()` calls the pure `MenuBar.validateForSave(context)` method and shows a user-friendly error dialog if validation fails.

‎docs/issues/issue-80.md‎

Lines changed: 1 addition & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,3 @@
11
# Issue #80: GUI InOut Validation
22

3-
**Status:** ✅ Implemented (2026-02-06)
4-
**Branch:** `copilot/add-gui-validation-in-editor`
5-
6-
## Problem
7-
8-
Users could save railway networks with 0 InOut elements through the GUI editor. The file was written successfully, but could not be reloaded (XML validation in XMLContextFactory requires ≥ 1 InOut since PR #356 bidirectional operation). Result: confusing "saved but can't be loaded" experience.
9-
10-
## Implementation
11-
12-
### Validation method — `MenuBar.validateForSave(context)`
13-
14-
A pure companion-object function added to `MenuBar`:
15-
16-
```kotlin
17-
fun validateForSave(context: EditingContext): Boolean =
18-
context.getInOuts().size >= XMLContextFactory.MIN_INOUT_ELEMENTS
19-
```
20-
21-
### Pre-save check — `MenuBar.performSave()`
22-
23-
Called before every save attempt. On failure, shows an error dialog and returns `false`:
24-
25-
```
26-
Cannot Save - Insufficient InOut Elements
27-
Railway network must have at least 1 InOut element (entry and exit point).
28-
Current count: 0
29-
```
30-
31-
All three save paths go through `performSave()`:
32-
1. File > Save → `SaveAction`
33-
2. File > Save As... → `SaveAsAction`
34-
3. Window close → `triggerSave()`
35-
36-
### Constant alignment
37-
38-
`MenuBar` uses `XMLContextFactory.MIN_INOUT_ELEMENTS` (= 1) — no separate constant. This avoids drift between the GUI and XML validation layers.
39-
40-
## Validation flow
41-
42-
```
43-
User: Save
44-
→ MenuBar.performSave(file)
45-
→ validateForSave(context)
46-
→ false: show error dialog, return false (save blocked)
47-
→ true: editingContextFactory.saveContext(...)
48-
→ success: update tracker, show status bar message
49-
→ failure: show IO error dialog
50-
```
51-
52-
## Test coverage
53-
54-
`InOutSaveValidationTest` (4 unit tests, no EDT, no reflection):
55-
56-
| InOut count | Expected | Test |
57-
|-------------|----------|------------------------------|
58-
| 0 | false | `contextWith0InOutsCannotBeSaved` |
59-
| 1 | true | `contextWith1InOutCanBeSaved` |
60-
| 2 | true | `contextWith2InOutsCanBeSaved` |
61-
| 3+ | true | `contextWith3InOutsCanBeSaved` |
62-
63-
Tests extend `KoinTestBase` and call `MenuBar.validateForSave(context)` directly.
64-
65-
## Related
66-
67-
- PR #356 — bidirectional train operation (allows single-InOut networks)
68-
- XMLContextFactory — enforces same constraint at XML load time
69-
- Issue #258 — future comprehensive validation framework
3+
Renamed to [`issue_80.md`](issue_80.md) for consistency with the directory naming scheme.

‎docs/issues/issue_80.md‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# Issue #80: GUI InOut Validation
2+
3+
**Status:** Implemented (2026-02-06)
4+
**Branch:** `copilot/add-gui-validation-in-editor`
5+
6+
## Problem
7+
8+
Users could save railway networks with 0 InOut elements through the GUI editor. The file was written successfully, but could not be reloaded (XML validation in XMLContextFactory requires >= 1 InOut since PR #356 bidirectional operation). Result: confusing "saved but can't be loaded" experience.
9+
10+
## Implementation
11+
12+
### Validation method — `MenuBar.validateForSave(context)`
13+
14+
A pure companion-object function added to `MenuBar`:
15+
16+
```kotlin
17+
fun validateForSave(context: EditingContext): Boolean =
18+
context.getInOuts().size >= XMLContextFactory.MIN_INOUT_ELEMENTS
19+
```
20+
21+
### Pre-save check — `MenuBar.performSave()`
22+
23+
Called before every save attempt. On failure, shows an error dialog and returns `false`:
24+
25+
```
26+
Cannot Save - Insufficient InOut Elements
27+
Railway network must have at least 1 InOut elements (entry and exit points).
28+
Current count: 0
29+
InOut elements define where trains enter and exit the railway network.
30+
Please add more InOut elements before saving.
31+
```
32+
33+
All three save paths go through `performSave()`:
34+
1. File > Save → `SaveAction`
35+
2. File > Save As... → `SaveAsAction`
36+
3. Window close → `triggerSave()`
37+
38+
### Constant alignment
39+
40+
`MenuBar` uses `XMLContextFactory.MIN_INOUT_ELEMENTS` (= 1) — no separate constant. This avoids drift between the GUI and XML validation layers.
41+
42+
## Validation flow
43+
44+
```
45+
User: Save
46+
-> MenuBar.performSave(file)
47+
-> validateForSave(context)
48+
-> false: show error dialog, return false (save blocked)
49+
-> true: editingContextFactory.saveContext(...)
50+
-> success: update tracker, show status bar message
51+
-> failure: show IO error dialog
52+
```
53+
54+
## Test coverage
55+
56+
`InOutSaveValidationTest` (4 unit tests, no EDT, no reflection):
57+
58+
| InOut count | Expected | Test |
59+
|-------------|----------|------------------------------|
60+
| 0 | false | `contextWith0InOutsCannotBeSaved` |
61+
| 1 | true | `contextWith1InOutCanBeSaved` |
62+
| 2 | true | `contextWith2InOutsCanBeSaved` |
63+
| 3+ | true | `contextWith3InOutsCanBeSaved` |
64+
65+
Tests extend `KoinTestBase` and call `MenuBar.validateForSave(context)` directly.
66+
67+
## Related
68+
69+
- PR #356 — bidirectional train operation (allows single-InOut networks)
70+
- XMLContextFactory — enforces same constraint at XML load time
71+
- Issue #258 — future comprehensive validation framework

0 commit comments

Comments
 (0)