From c84f39ca294800e3232f86d2d3f975c39d0b3171 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:19:16 +0200 Subject: [PATCH 01/18] set up CI --- .github/workflows/maven.yml | 27 +++++++++++++++++++++++++++ .maven-settings.xml | 9 +++++++++ 2 files changed, 36 insertions(+) create mode 100644 .github/workflows/maven.yml create mode 100644 .maven-settings.xml diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml new file mode 100644 index 000000000..84d2370a0 --- /dev/null +++ b/.github/workflows/maven.yml @@ -0,0 +1,27 @@ +name: Java CI with Maven + +on: + push: + branches: [ "develop" ] + pull_request: + branches: [ "develop" ] + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up JDK 23 + uses: actions/setup-java@v4 + with: + java-version: '23' + distribution: 'temurin' + cache: maven + - name: Build with Maven + run: mvn -B package --settings .maven-settings.xml --file pom.xml + - name: Run tests with Maven + run: mvn test --settings .maven-settings.xml --file pom.xml + - name: Run Main class with Maven Exec + run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.svg.Main --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml + - name: Update dependency graph + uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 diff --git a/.maven-settings.xml b/.maven-settings.xml new file mode 100644 index 000000000..5370d863d --- /dev/null +++ b/.maven-settings.xml @@ -0,0 +1,9 @@ + + + + github + ${{ secrets.GITHUB_ACTOR }} + ${{ secrets.GITHUB_TOKEN }} + + + From f03c6169bb429bdd27d77471800bb48ecdcb07f9 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:21:21 +0200 Subject: [PATCH 02/18] fix --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 84d2370a0..92077c2c3 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -20,7 +20,7 @@ jobs: - name: Build with Maven run: mvn -B package --settings .maven-settings.xml --file pom.xml - name: Run tests with Maven - run: mvn test --settings .maven-settings.xml --file pom.xml + run: mvn test --settings .maven-settings.xml --file pom.xml - name: Run Main class with Maven Exec run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.svg.Main --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml - name: Update dependency graph From f40d9db1980a815ac5bc7ad03802cf6cc8110ee1 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:25:04 +0200 Subject: [PATCH 03/18] github tokens correct --- .github/workflows/maven.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 92077c2c3..d202b53a0 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -17,6 +17,12 @@ jobs: java-version: '23' distribution: 'temurin' cache: maven + - name: Configure GitHub Packages credentials + run: | + echo "\n \n \n github\n ${GITHUB_ACTOR}\n ${GITHUB_TOKEN}\n \n \n" > $GITHUB_WORKSPACE/.maven-settings.xml + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_ACTOR: ${{ github.actor }} - name: Build with Maven run: mvn -B package --settings .maven-settings.xml --file pom.xml - name: Run tests with Maven From 59f5ccb0f4db1d27ba5421df2eb1ef126adeb502 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:26:54 +0200 Subject: [PATCH 04/18] github tokens correct 2 --- .github/workflows/maven.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index d202b53a0..e66736683 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -19,7 +19,17 @@ jobs: cache: maven - name: Configure GitHub Packages credentials run: | - echo "\n \n \n github\n ${GITHUB_ACTOR}\n ${GITHUB_TOKEN}\n \n \n" > $GITHUB_WORKSPACE/.maven-settings.xml + cat < $GITHUB_WORKSPACE/.maven-settings.xml + + + + github + ${GITHUB_ACTOR} + ${GITHUB_TOKEN} + + + + EOF env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_ACTOR: ${{ github.actor }} From 56ab3bdaa5fc1d19c92c3f54bbb1f84d4545d084 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:30:26 +0200 Subject: [PATCH 05/18] github tokens correct 3 --- .github/workflows/maven.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index e66736683..4a78e5817 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -25,13 +25,13 @@ jobs: github ${GITHUB_ACTOR} - ${GITHUB_TOKEN} + ${GH_PACKAGES_TOKEN} EOF env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_PACKAGES_TOKEN: ${{ secrets.GH_PACKAGES_TOKEN }} GITHUB_ACTOR: ${{ github.actor }} - name: Build with Maven run: mvn -B package --settings .maven-settings.xml --file pom.xml From 19d3105836d8517a1275fa3e3b9a5a138327dbf5 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:37:06 +0200 Subject: [PATCH 06/18] pls work --- .github/workflows/maven.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 4a78e5817..67fa0705d 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -19,24 +19,25 @@ jobs: cache: maven - name: Configure GitHub Packages credentials run: | - cat < $GITHUB_WORKSPACE/.maven-settings.xml + mkdir -p ~/.m2 + cat > ~/.m2/settings.xml < github ${GITHUB_ACTOR} - ${GH_PACKAGES_TOKEN} + ${GITHUB_TOKEN} EOF env: - GH_PACKAGES_TOKEN: ${{ secrets.GH_PACKAGES_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_ACTOR: ${{ github.actor }} - name: Build with Maven - run: mvn -B package --settings .maven-settings.xml --file pom.xml + run: mvn -B package --file pom.xml - name: Run tests with Maven - run: mvn test --settings .maven-settings.xml --file pom.xml + run: mvn test --file pom.xml - name: Run Main class with Maven Exec run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.svg.Main --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml - name: Update dependency graph From de970e07e946e8822a2d0c9dc1e9b2e42668a37d Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:40:30 +0200 Subject: [PATCH 07/18] pls work 2 --- .github/workflows/maven.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 67fa0705d..f08f59ec0 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -34,11 +34,11 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_ACTOR: ${{ github.actor }} - - name: Build with Maven - run: mvn -B package --file pom.xml - - name: Run tests with Maven - run: mvn test --file pom.xml + - name: Build jhotdraw-samples-misc with Maven + run: mvn -B package --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml + - name: Run tests for jhotdraw-samples-misc + run: mvn test --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml - name: Run Main class with Maven Exec - run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.svg.Main --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml + run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.draw.Main --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml - name: Update dependency graph uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 From 64e654f8574ada6534b8a5b69a6121f465c7746e Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:42:44 +0200 Subject: [PATCH 08/18] pls work 3 --- pom.xml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 5f6c7eef5..1a66ef2f7 100644 --- a/pom.xml +++ b/pom.xml @@ -8,13 +8,7 @@ JHotDraw - - - github - GitHub external Packages - https://maven.pkg.github.com/sweat-tek/MavenRepository - - + GNU Library or Lesser General Public License (LGPL) V2.1 From 4cdaf71d5d23d296608e53283365f33509f30cde Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:45:46 +0200 Subject: [PATCH 09/18] better --- .github/workflows/maven.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index f08f59ec0..5effd456f 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -34,11 +34,11 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_ACTOR: ${{ github.actor }} - - name: Build jhotdraw-samples-misc with Maven - run: mvn -B package --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml - - name: Run tests for jhotdraw-samples-misc - run: mvn test --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml - - name: Run Main class with Maven Exec - run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.draw.Main --file jhotdraw-samples/jhotdraw-samples-misc/pom.xml + - name: Build entire project with Maven + run: mvn -B clean install --file pom.xml + - name: Run tests + run: mvn test --file pom.xml + - name: Run jhotdraw-samples-misc application + run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.draw.Main -pl jhotdraw-samples/jhotdraw-samples-misc - name: Update dependency graph uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 From 1d48628828046ca9d6ffa26abb37a6163998bf8f Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:47:31 +0200 Subject: [PATCH 10/18] better2 --- .github/workflows/maven.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 5effd456f..48330541b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -38,7 +38,5 @@ jobs: run: mvn -B clean install --file pom.xml - name: Run tests run: mvn test --file pom.xml - - name: Run jhotdraw-samples-misc application - run: mvn exec:java -Dexec.mainClass=org.jhotdraw.samples.draw.Main -pl jhotdraw-samples/jhotdraw-samples-misc - name: Update dependency graph uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 From 57565f84a671aa88fcb812d29cb61ad2270ca5d5 Mon Sep 17 00:00:00 2001 From: Jakub Potocky Date: Sun, 10 May 2026 15:59:54 +0200 Subject: [PATCH 11/18] remove graph --- .github/workflows/maven.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 48330541b..6205bf836 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -38,5 +38,3 @@ jobs: run: mvn -B clean install --file pom.xml - name: Run tests run: mvn test --file pom.xml - - name: Update dependency graph - uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 From 8b78d511f9ae7796d505cc0b1cb56a160861ecd3 Mon Sep 17 00:00:00 2001 From: alsiepl Date: Thu, 4 Jun 2026 14:01:49 +0200 Subject: [PATCH 12/18] al --- .vscode/launch.json | 23 ++ AlansWork/alans.md | 452 ++++++++++++++++++++++ AlansWork/alansPortfolioWork.md | 136 +++++++ concept-location-notes.html | 664 ++++++++++++++++++++++++++++++++ concept-location-notes.md | 139 +++++++ 5 files changed, 1414 insertions(+) create mode 100644 .vscode/launch.json create mode 100644 AlansWork/alans.md create mode 100644 AlansWork/alansPortfolioWork.md create mode 100644 concept-location-notes.html create mode 100644 concept-location-notes.md diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 000000000..2b3fe865d --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,23 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "type": "java", + "name": "Debug JHotDraw Draw Sample", + "request": "launch", + "mainClass": "org.jhotdraw.samples.draw.Main", + "projectName": "jhotdraw-samples-misc", + "classPaths": [ + "${workspaceFolder}/jhotdraw-samples/jhotdraw-samples-misc/target/classes", + "${workspaceFolder}/jhotdraw-core/target/classes", + "${workspaceFolder}/jhotdraw-actions/target/classes", + "${workspaceFolder}/jhotdraw-app/target/classes", + "${workspaceFolder}/jhotdraw-api/target/classes", + "${workspaceFolder}/jhotdraw-gui/target/classes", + "${workspaceFolder}/jhotdraw-utils/target/classes", + "${workspaceFolder}/jhotdraw-xml/target/classes", + "${workspaceFolder}/jhotdraw-datatransfer/target/classes" + ] + } + ] +} diff --git a/AlansWork/alans.md b/AlansWork/alans.md new file mode 100644 index 000000000..20244770a --- /dev/null +++ b/AlansWork/alans.md @@ -0,0 +1,452 @@ +# Concept Location via Dynamic Analysis — JHotDraw + +> **Goal of this exercise** +> Use the IDE Debugger to find — at runtime — every class that participates in the **Automatic Selection** feature of JHotDraw, and produce an *initial set of classes* table for the portfolio. + +--- + +## Part 1 — One-Time Setup + +Do these once before you start. + +### Step 1. Build the project + +Open a PowerShell terminal in VS Code (`` Ctrl+` ``) and run: + +```powershell +cd "c:\Users\alans\Desktop\SM\JHotDraw" +mvn install -DskipTests +``` + +Wait until you see `BUILD SUCCESS`. + +### Step 2. Confirm the launch configuration exists + +The file [.vscode/launch.json](.vscode/launch.json) must contain a configuration named **"Debug JHotDraw Draw Sample"**. If it doesn't, create it with this content: + +```json +{ + "version": "0.2.0", + "configurations": [ + { + "type": "java", + "name": "Debug JHotDraw Draw Sample", + "request": "launch", + "mainClass": "org.jhotdraw.samples.draw.Main", + "projectName": "jhotdraw-samples-misc" + } + ] +} +``` + +### Step 3. Learn the four debugger keys + +| Key | What it does | +|---|---| +| **F5** | Continue running until next breakpoint | +| **F10** | Step Over — execute current line, stay in same method | +| **F11** | Step Into — go inside the method called on this line | +| **Shift+F11** | Step Out — finish current method and return to caller | + +Memorise these panels in the **Run and Debug** view (`Ctrl+Shift+D`): +- **Variables** — current values of local variables and fields +- **Call Stack** — chain of methods that led to the current line *(this is what you copy into your portfolio table)* +- **Breakpoints** — list of all breakpoints (you can disable/remove them here) + +--- + +## Part 2 — Scenario A: Select All + +**Goal:** Trace what happens when the user clicks **Edit → Select All**. + +### Step 1. Set the breakpoints + +Open each file below, click in the gutter (left of the line number) on the listed line. A red dot appears. + +| File | Line | Method | +|---|---|---| +| [SelectAllAction.java](jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/SelectAllAction.java#L74) | 74 | `actionPerformed()` | +| [DefaultDrawingView.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/DefaultDrawingView.java#L844) | 844 | `selectAll()` | +| [DefaultDrawingView.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/DefaultDrawingView.java#L1062) | 1062 | `fireSelectionChanged()` | + +### Step 2. Launch the app + +Press **F5**. The JHotDraw window opens. + +### Step 3. Create test data + +Inside the running app: +1. Click the **Rectangle** tool in the toolbar. +2. Draw 2 rectangles on the canvas. +3. Click the **Ellipse** tool. +4. Draw 1 ellipse. + +### Step 4. Trigger the feature + +Click **Edit → Select All** in the menu bar. + +### Step 5. Observe in the debugger + +The debugger pauses at line 74 of `SelectAllAction`. + +1. Look at the **Variables** panel — note the value of `c` (it should be a `DefaultDrawingView`). +2. Press **F10** until the highlighted line is `((EditableComponent) c).selectAll();`. +3. Press **F11** — you jump into `DefaultDrawingView.selectAll()`. +4. In the **Variables** panel expand `selectedFigures`. It is currently empty. +5. Press **F10** through the `for` loop. After each iteration, watch `selectedFigures` grow by one figure. +6. After the loop, the highlighted line is `fireSelectionChanged(oldSelection, newSelection);`. Press **F11**. +7. You are now inside `fireSelectionChanged()`. Look at the **Call Stack** panel — it shows the full chain. + +### Step 6. Record the call stack + +Open the **Call Stack** panel and write down every class name from top to bottom. You should see: + +``` +DefaultDrawingView.fireSelectionChanged() ← current +DefaultDrawingView.selectAll() +SelectAllAction.actionPerformed() +... (Swing menu plumbing — ignore these) +``` + +### Step 7. Stop + +Press the red **Stop** button (or **Shift+F5**). + +### What you just confirmed + +``` +SelectAllAction.actionPerformed() + └─► DefaultDrawingView.selectAll() + - selectedFigures.clear() + - for each Figure in drawing.getChildren(): + if figure.isSelectable() → selectedFigures.add(figure) + - invalidateHandles() + - fireSelectionChanged(old, new) + └─► FigureSelectionEvent → all FigureSelectionListeners + - repaint() +``` + +--- + +## Part 3 — Scenario B: Deselect All (Clear Selection) + +**Goal:** Trace what happens when the user clicks **Edit → Clear Selection**. + +### Step 1. Disable the Scenario A breakpoints + +In the **Breakpoints** panel, uncheck the three breakpoints from Scenario A (don't delete them — you may want them again). + +### Step 2. Set the new breakpoints + +| File | Line | Method | +|---|---|---| +| [ClearSelectionAction.java](jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/ClearSelectionAction.java#L74) | 74 | `actionPerformed()` | +| [DefaultDrawingView.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/DefaultDrawingView.java#L862) | 862 | `clearSelection()` | +| [DefaultDrawingView.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/DefaultDrawingView.java#L1062) | 1062 | `fireSelectionChanged()` | + +### Step 3. Launch the app + +Press **F5**. + +### Step 4. Create test data and select something + +1. Draw 2 rectangles. +2. Click the **Selection** tool (the arrow). +3. Click on one rectangle to select it. (You will see selection handles appear around it.) + +> Without this step the menu item is disabled and your breakpoint will never hit. + +### Step 5. Trigger the feature + +Click **Edit → Clear Selection**. + +### Step 6. Observe in the debugger + +1. Debugger pauses in `ClearSelectionAction.actionPerformed()`. Press **F10** until the highlighted line is `((EditableComponent) c).clearSelection();`. +2. Press **F11** — you jump into `DefaultDrawingView.clearSelection()`. +3. In the **Variables** panel, `selectedFigures` contains 1 figure. +4. Press **F10** past `selectedFigures.clear();` — now it is empty. +5. Press **F11** at `fireSelectionChanged(...)`. +6. In the **Call Stack** panel, record every class. + +### Step 7. Stop + +Press **Shift+F5**. + +### What you just confirmed + +``` +ClearSelectionAction.actionPerformed() + └─► DefaultDrawingView.clearSelection() + - if selectedFigures not empty: + - oldSelection = copy of selectedFigures + - selectedFigures.clear() + - invalidateHandles() + - fireSelectionChanged(old, empty) + └─► FigureSelectionEvent → all FigureSelectionListeners +``` + +--- + +## Part 4 — Scenario C: Select Same + +**Goal:** Trace what happens when the user clicks the **Select Same** toolbar button. + +### Step 1. Disable the Scenario B breakpoints + +Uncheck them in the **Breakpoints** panel. + +### Step 2. Set the new breakpoints + +| File | Line | Method | +|---|---|---| +| [SelectSameAction.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/action/SelectSameAction.java#L36) | 36 | `actionPerformed()` | +| [SelectSameAction.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/action/SelectSameAction.java#L41) | 41 | `selectSame()` | +| [DefaultDrawingView.java](jhotdraw-core/src/main/java/org/jhotdraw/draw/DefaultDrawingView.java#L742) | 742 | `addToSelection(Figure)` | + +### Step 3. Launch the app + +Press **F5**. + +### Step 4. Create test data and pre-select + +1. Draw **2 rectangles** and **1 ellipse**. +2. Click the **Selection** tool. +3. Click **one rectangle** (only one figure must be selected for the toolbar button to enable). + +### Step 5. Trigger the feature + +Click the **Select Same** button on the toolbar (its tooltip reads "Select Same"). + +### Step 6. Observe in the debugger + +1. Debugger pauses in `SelectSameAction.actionPerformed()`. Press **F11** to step into `selectSame()`. +2. After the first `for` loop, expand `selectedClasses` in **Variables** — it contains exactly one class: `RectangleFigure`. +3. Press **F10** through the second loop. For each `Figure f`: + - When `f` is a rectangle → execution enters the `if` block. Press **F11** at `getView().addToSelection(f)` to step into `DefaultDrawingView.addToSelection()`. + - When `f` is the ellipse → the `if` is skipped (the ellipse class is not in `selectedClasses`). +4. Inside `addToSelection()`, watch `selectedFigures` grow. +5. Each time, `fireSelectionChanged()` is invoked → check the **Call Stack**. + +### Step 7. Stop + +Press **Shift+F5**. + +### What you just confirmed + +``` +SelectSameAction.actionPerformed() + └─► SelectSameAction.selectSame() + - selectedClasses ← classes of getView().getSelectedFigures() + - for each Figure f in getDrawing().getChildren(): + if selectedClasses.contains(f.getClass()) + └─► DefaultDrawingView.addToSelection(f) + - selectedFigures.add(figure) + - figure.createHandles(detailLevel) + - fireSelectionChanged(old, new) + └─► FigureSelectionEvent → all FigureSelectionListeners +``` + +--- + +## Part 5 — Portfolio Deliverable: Initial Set of Classes + +| Domain Class | Responsibility | +|---|---| +| `SelectAllAction` | Controller entry point for the "Select All" menu item; resolves the focused `EditableComponent` and invokes its `selectAll()` method. | +| `ClearSelectionAction` | Controller entry point for the "Clear Selection" menu item; resolves the focused `EditableComponent` and invokes its `clearSelection()` method. | +| `SelectSameAction` | Controller entry point for the "Select Same" toolbar action; collects the classes of currently selected figures and adds every figure of those classes to the selection. | +| `AbstractSelectionAction` | Abstract base class for `SelectAllAction` and `ClearSelectionAction`; manages action enablement based on the target component's selection state. | +| `AbstractSelectedAction` | Abstract base class for `SelectSameAction`; listens to `DrawingEditor` for the active view and enables the action only when at least one figure is selected. | +| `EditableComponent` | Contract interface declaring `selectAll()` and `clearSelection()`; implemented by `DefaultDrawingView` to make the drawing canvas behave as an editable component. | +| `DrawingView` | Contract interface declaring the full selection API: `addToSelection()`, `removeFromSelection()`, `selectAll()`, `clearSelection()`, `getSelectedFigures()`. | +| `DefaultDrawingView` | Core domain class that owns the `selectedFigures` set and implements `selectAll()`, `clearSelection()`, and `addToSelection()`; fires `FigureSelectionEvent` after every change. | +| `Drawing` | Domain model that holds all figures; provides `getChildren()` which is iterated by `selectAll()` and `selectSame()`. | +| `Figure` | Domain entity representing each shape; queried via `isSelectable()` (Select All) and `getClass()` (Select Same) to decide whether it joins the selection. | +| `DrawingEditor` | Mediator that exposes the active `DrawingView` to figure-aware actions such as `SelectSameAction`. | +| `FigureSelectionEvent` | Event object carrying the old and new selection sets; created by `DefaultDrawingView.fireSelectionChanged()`. | +| `FigureSelectionListener` | Observer interface; every registered listener receives `selectionChanged(FigureSelectionEvent)` whenever the selection changes. | +| `AbstractAttributeEditorHandler` | Observer that reacts to selection changes by refreshing attribute editor panels (color, stroke, font, etc.). | +| `SelectionComponentDisplayer` | Observer that shows or hides toolbar UI components depending on whether figures are selected. | +| `SelectionComponentRepainter` | Observer that repaints toolbar UI components to reflect the attributes of the new selection. | + +--- + +## Part 6 — Troubleshooting + +| Symptom | Cause | Fix | +|---|---|---| +| Breakpoint never hits | The menu/button is disabled, so `actionPerformed` is never called | Pre-select a figure (Scenarios B & C); make sure the canvas has focus | +| "Source not found" when stepping | Module not built | Re-run `mvn install -DskipTests` | +| App won't launch | Wrong `projectName` in launch.json | Confirm it is `jhotdraw-samples-misc` | +| Too many breakpoints firing | Old breakpoints from prior scenario still active | Uncheck them in the **Breakpoints** panel | +| Lost in the call stack | Stepped into Swing internals | Press **Shift+F11** repeatedly until you are back in a `org.jhotdraw.*` frame | + +--- + +## Part 7 — Key Take-aways + +- **Controllers** in JHotDraw are `*Action` classes under `jhotdraw-actions` (generic) and `jhotdraw-core/.../draw/action` (drawing-specific). +- The `EditableComponent` interface decouples the generic edit menu actions from the concrete drawing canvas (`DefaultDrawingView`). +- `DefaultDrawingView` is the **single source of truth** for the current selection (`selectedFigures` set). +- Selection changes are propagated through the **Observer pattern** via `FigureSelectionEvent` / `FigureSelectionListener`, enabling toolbars and attribute editors to react automatically. +- `SelectSameAction` differs structurally from the other two: it extends `AbstractSelectedAction` (drawing-aware, requires non-empty selection) instead of `AbstractSelectionAction` (generic Swing-component-aware). + + + +--- + +# Part 8 Change Impact Analysis (Marking Algorithm) + +> **Goal of this chapter** +> Take the classes you found by concept location and decide, one by one, which ones a *real change request* would affect. We follow the marking algorithm from Rajlich, *Software Engineering: The Current Practice*, Figure 7.9. + +> **Concrete change request used in this exercise** +> *After Select All / Deselect All / Select Same, show a "N figure(s) selected" message in a status bar at the bottom of the Draw window, and keep it in sync with every later selection change.* + +--- + +## Step 1 Understand the four marks + +Every class will end up with exactly one of these labels: + +| Mark | Meaning | +|---|---| +| **BLANK** | Not yet visited. | +| **NEXT** | A neighbor of an already-marked class; it has to be inspected next. | +| **UNCHANGED** | After inspection: the change request does NOT affect this class. Dead end. | +| **PROPAGATES** | After inspection: the change passes through this class (it is on the path) but the class itself does not need source edits. | +| **CHANGED** | After inspection: this class must actually be edited. | + +The **Estimated Impact Set** at the end = every class marked **CHANGED** plus every class marked **PROPAGATES**. + +--- + +## Step 2 Seed the algorithm with the three starting classes + +These are the classes concept location already gave you. Mark them **CHANGED** without inspection: + +| Starting class | Why it is the entry point | +|---|---| +| `org.jhotdraw.action.edit.SelectAllAction` | Controller for *Select All* | +| `org.jhotdraw.action.edit.ClearSelectionAction` | Controller for *Deselect All* | +| `org.jhotdraw.draw.action.SelectSameAction` | Controller for *Select Same* | + +> Note on the package name: the assignment text says `org.jhotdraw.app.action.edit`, but in this codebase the package was renamed to `org.jhotdraw.action.edit` (module `jhotdraw-actions`). Use the real names you see in the source. + +Now mark every BLANK neighbor of these three as **NEXT**. + +--- + +## Step 3 How to find neighbors of a class (in VS Code) + +For each class you are inspecting, do this every time: + +1. Open the `.java` file. +2. Look at the `extends` / `implements` clauses those are *inheritance neighbors*. +3. Look at every `import org.jhotdraw.*` line those are *reference neighbors*. +4. Right-click the class name in the editor **Find All References** that gives you *callers*. +5. Inside the class body, look at every method call on a non-local variable those are *callees*. + +Write the neighbors down before moving on. **Do not invent neighbors you have not verified in the source.** + +--- + +## Step 4 How to inspect one NEXT class + +For each NEXT class, ask yourself this single question: + +> *"To satisfy the change request (show selection count in a status bar), do I need to edit THIS file?"* + +Then pick exactly one of three answers and write a one-sentence reason: + +- **UNCHANGED** "No edits, and the change does not flow through here." +- **PROPAGATES** "No edits, but my class is on the call/observer path between an edited class and another class that may need editing." +- **CHANGED** "Yes, I have to open this file and modify it." + +If the answer is **PROPAGATES** or **CHANGED**, mark every still-BLANK neighbor of that class as **NEXT**. +If the answer is **UNCHANGED**, stop its neighbors are NOT added. + +Keep doing Step 4 until there are zero NEXT classes left. + +--- + +## Step 5 Apply the algorithm to JHotDraw (visit-order trace) + +Visit the classes in this order. Each row shows the mark and the one-sentence justification. + +| # | Class | Mark | Reason | +|---|---|---|---| +| 1 | `SelectAllAction` | CHANGED | Seeded entry point of *Select All*. | +| 2 | `ClearSelectionAction` | CHANGED | Seeded entry point of *Deselect All*. | +| 3 | `SelectSameAction` | CHANGED | Seeded entry point of *Select Same*. | +| 4 | `AbstractSelectionAction` | UNCHANGED | Parent of #1 and #2; only manages the action's enabled state, no selection-count work happens here. | +| 5 | `AbstractSelectedAction` | UNCHANGED | Parent of #3; same story uses `FigureSelectionEvent` only to refresh enablement. | +| 6 | `EditableComponent` | UNCHANGED | Already exposes everything the status bar needs (`SELECTION_EMPTY_PROPERTY`); no new method required. | +| 7 | `DrawingEditor` | PROPAGATES | The new status-bar listener has to follow the *active view* via `ACTIVE_VIEW_PROPERTY`, so the change passes through this interface but the interface itself does not change. Adds neighbor `DrawingView`. | +| 8 | `Figure` | UNCHANGED | The CR is about counting figures, not about per-figure behavior. | +| 9 | `DefaultApplicationModel` | UNCHANGED | Only registers `SelectAllAction.ID` / `ClearSelectionAction.ID` in the action map wiring stays the same. | +| 10 | `DefaultDrawingEditor` | UNCHANGED | Wires keystrokes for `SelectAllAction.ID`; key bindings are unchanged. | +| 11 | `ButtonFactory` | UNCHANGED | Builds the toolbar button for `SelectSameAction`; button construction is unchanged. | +| 12 | `DefaultMenuBuilder` | UNCHANGED | Only assembles the Edit menu by ID; menu structure is unchanged. | +| 13 | `DrawingPanel` (Draw sample) | UNCHANGED | Only a layout host for tools/attributes; status bar lives one level up in `DrawView`. | +| 14 | `DrawingView` (interface) | UNCHANGED | API already provides `getSelectionCount()`, `getSelectedFigures()`, `addFigureSelectionListener()`. Adds neighbor `DrawView`. | +| 15 | `DrawView` (Draw sample) | CHANGED | This is the `AbstractView` subclass that owns the scroll pane + `DrawingView`; the new status-bar component, label, and `FigureSelectionListener` go here. | +| 16 | `AbstractView` | UNCHANGED | Generic life-cycle base; the status bar is added in the concrete `DrawView`, not in the base class. | + +When you finish row 16 there are no more NEXT classes **STOP**. + +--- + +## Step 6 Write down the Estimated Impact Set + +The Estimated Impact Set is just the rows whose mark is **CHANGED** or **PROPAGATES**: + +| Mark | Class | Module | +|---|---|---| +| CHANGED | `SelectAllAction` | `jhotdraw-actions` | +| CHANGED | `ClearSelectionAction` | `jhotdraw-actions` | +| CHANGED | `SelectSameAction` | `jhotdraw-core` | +| CHANGED | `DrawView` | `jhotdraw-samples-misc` | +| PROPAGATES | `DrawingEditor` | `jhotdraw-core` | + +**Impact set size: 5 existing classes** (plus one new helper class e.g. `SelectionCountStatusLabel implements FigureSelectionListener` that you will *create*; new classes are not part of the marking algorithm). + +--- + +## Step 7 Group the visited classes by package (Table 1) + +| Package | # classes visited | Comments | +|---|---:|---| +| `org.jhotdraw.action.edit` (module `jhotdraw-actions`) | 3 | App-layer Edit-menu controllers. Hosts `SelectAllAction`, `ClearSelectionAction` and their abstract base. They are component-agnostic: they delegate to whatever focused `EditableComponent` exists, which is what makes the same Edit menu work for both the canvas and a `JTextComponent`. | +| `org.jhotdraw.api.gui` (module `jhotdraw-api`) | 1 | Defines `EditableComponent` (`selectAll`, `clearSelection`, `SELECTION_EMPTY_PROPERTY`). It is the seam between the generic edit actions and the concrete canvas the reason the change does not propagate further into the action hierarchy. | +| `org.jhotdraw.draw` (module `jhotdraw-core`) | 2 | Core drawing framework. `DrawingEditor` / `DefaultDrawingEditor` track the active `DrawingView`, own the action map, and bind keystrokes the bridge between the *Select Same* path and the underlying selection state. | +| `org.jhotdraw.draw.action` (module `jhotdraw-core`) | 2 | Drawing-aware action layer. `SelectSameAction` (3rd entry point) plus its abstract base `AbstractSelectedAction`, which already listens for `FigureSelectionEvent` to keep its enabled state in sync confirming the Observer wiring needed by the new status bar is already in place. | +| `org.jhotdraw.draw.figure` (module `jhotdraw-core`) | 1 | Domain entity package containing `Figure`. Visited as a neighbor of `SelectSameAction`; UNCHANGED because the CR is about counting, not per-figure behavior. | +| `org.jhotdraw.app` (module `jhotdraw-app`) | 3 | Application shell. `DefaultApplicationModel` registers the actions, `DefaultMenuBuilder` builds the Edit menu, `AbstractView` is the base class of `DrawView`. None change the CR is satisfied at the View level. | +| `org.jhotdraw.gui.action` (module `jhotdraw-gui`) | 1 | `ButtonFactory` constructs the SVG-style selection toolbar. UNCHANGED the new behavior is not button-construction-time. | +| `org.jhotdraw.samples.draw` (module `jhotdraw-samples-misc`) | 2 | The Draw sample itself. `DrawingPanel` is a layout host (UNCHANGED); `DrawView` owns the scroll pane and the `DrawingView` and is the natural home for the new status bar (CHANGED). | + +**Total classes visited: 15** (3 seeded + 12 inspected). + +--- + +## Step 8 Sanity checks before you submit + +Tick these one by one: + +- [ ] Every class in the Impact Set was actually inspected (not assumed). +- [ ] Every class is counted exactly once even if it was reached as a neighbor of more than one other class. +- [ ] Each row in the visit-order trace has its own one-sentence justification. +- [ ] The CR is stated explicitly at the top of the chapter (so the marks can be re-evaluated if the CR changes). +- [ ] Any class whose dependencies you could not verify in the source is flagged as such, instead of being guessed. + +--- + +## Step 9 Key take-aways from this analysis + +- JHotDraw absorbs this change cheaply because the **Observer pattern is already in place**: `FigureSelectionEvent` / `FigureSelectionListener` over `DrawingView` publishes exactly what the new status bar needs. +- The two paths split cleanly along the layer boundary: + - **App-layer path** (Select All / Clear Selection) terminates at `EditableComponent` and never reaches the drawing core. + - **Draw-layer path** (Select Same) propagates through `DrawingEditor` to `DrawingView`, then stops because the API is already sufficient. +- The only real source edit beyond the three seeded actions lives in `DrawView` that is where the status-bar component, the listener registration, and the label-text update belong. diff --git a/AlansWork/alansPortfolioWork.md b/AlansWork/alansPortfolioWork.md new file mode 100644 index 000000000..e36813fa9 --- /dev/null +++ b/AlansWork/alansPortfolioWork.md @@ -0,0 +1,136 @@ +# Portfolio Work + +This document collects the portfolio deliverables for the Software Maintenance course. +Each chapter corresponds to one lab. + +--- + +# Chapter: ChangeReqLab — Change Request + +> **System under maintenance:** JHotDraw 7.6 (Java structured-graphics editor framework). +> **Selected feature:** Automatic Selection — covering three sub-features in the Edit menu / toolbar: +> - **Select All** — select every figure on the canvas. +> - **Deselect All (Clear Selection)** — empty the current selection. +> - **Select Same** — extend the current selection with every figure whose class matches a class already in the selection. + +## User stories + +The feature is described from the point of view of the end user of the Draw sample application, using the standard *"As a … I want … so that …"* template. Each story also lists its acceptance criteria. + +### US-1 — Select All + +> **As** a user editing a drawing, +> **I want** to select every figure on the canvas with a single command, +> **so that** I can apply an operation (move, delete, change attributes, copy) to the whole drawing without clicking each figure individually. + +**Trigger:** menu *Edit → Select All*, or the keyboard shortcut `Ctrl+A`. + +**Acceptance criteria:** +1. After the command, every selectable figure on the active canvas is part of the current selection. +2. Figures whose `isSelectable()` returns `false` (e.g. locked figures) are not added. +3. The selection handles are visible on every newly selected figure. +4. Any UI element that listens to selection changes (attribute panels, toolbar buttons) is refreshed. + +### US-2 — Deselect All (Clear Selection) + +> **As** a user editing a drawing, +> **I want** to clear the current selection in one step, +> **so that** I can start a fresh selection without having to click on empty canvas or unselect each figure individually. + +**Trigger:** menu *Edit → Clear Selection*. + +**Acceptance criteria:** +1. After the command, the current selection is empty. +2. All selection handles disappear from the canvas. +3. Actions whose enablement depends on a non-empty selection (e.g. *Select Same*) become disabled. +4. The command is a no-op (and visibly disabled) when the selection is already empty. + +### US-3 — Select Same + +> **As** a user editing a drawing that contains many shapes of different kinds, +> **I want** to extend the current selection to every figure of the same kind as the figures I already have selected, +> **so that** I can apply the same change (e.g. recolour all rectangles) to a whole category of shapes without picking them one by one. + +**Trigger:** *Select Same* button on the selection toolbar. + +**Acceptance criteria:** +1. The button is disabled when the current selection is empty. +2. After the command, the selection contains every figure on the canvas whose runtime class matches at least one class already in the previous selection. +3. Figures of other classes remain unselected. +4. The command works additively — figures already selected stay selected. + +--- + +## Change request statement (chosen for the impact-analysis lab) + +*After Select All / Deselect All / Select Same, the Draw sample window must display a status-bar message of the form `"N figure(s) selected"` reflecting the new selection size. The status bar must also stay in sync with any later selection change (mouse click, drag-rectangle, keyboard shortcut, etc.), not only with the three actions above.* + +## Why this change request + +- It exercises **all three** sub-features of the Automatic Selection feature, so concept location and impact analysis cover one coherent slice of the system. +- It is realistic — a status bar with the current selection count is a feature found in most graphical editors. +- It is non-trivial enough to propagate beyond the three controller classes (it reaches the View layer of the sample), but small enough to be feasible inside one lab. + +--- + +# Chapter: CLLab — Concept Location + +> **Technique used:** IDE Debugger (dynamic program analysis) on the Draw sample (`org.jhotdraw.samples.draw.Main`). +> Breakpoints were placed at suspected controller entry points; execution was followed via Step Over / Step Into; the **Call Stack** and **Variables** panels in VS Code were used to record every class that participates at runtime. +> Full step-by-step procedure is in [alans.md](alans.md) (Parts 1–4). + +## Initial set of classes — result of concept location + +| Domain Class | Responsibility | +|---|---| +| `SelectAllAction` | Controller entry point for the "Select All" menu item; resolves the focused `EditableComponent` and invokes its `selectAll()` method. | +| `ClearSelectionAction` | Controller entry point for the "Clear Selection" menu item; resolves the focused `EditableComponent` and invokes its `clearSelection()` method. | +| `SelectSameAction` | Controller entry point for the "Select Same" toolbar action; collects the classes of currently selected figures and adds every figure of those classes to the selection. | +| `AbstractSelectionAction` | Abstract base class for `SelectAllAction` and `ClearSelectionAction`; manages action enablement based on the target component's selection state. | +| `AbstractSelectedAction` | Abstract base class for `SelectSameAction`; listens to `DrawingEditor` for the active view and enables the action only when at least one figure is selected. | +| `EditableComponent` | Contract interface declaring `selectAll()` and `clearSelection()`; implemented by `DefaultDrawingView` to make the drawing canvas behave as an editable component. | +| `DrawingView` | Contract interface declaring the full selection API: `addToSelection()`, `removeFromSelection()`, `selectAll()`, `clearSelection()`, `getSelectedFigures()`. | +| `DefaultDrawingView` | Core domain class that owns the `selectedFigures` set and implements `selectAll()`, `clearSelection()`, and `addToSelection()`; fires `FigureSelectionEvent` after every change. | +| `Drawing` | Domain model that holds all figures; provides `getChildren()` which is iterated by `selectAll()` and `selectSame()`. | +| `Figure` | Domain entity representing each shape; queried via `isSelectable()` (Select All) and `getClass()` (Select Same) to decide whether it joins the selection. | +| `DrawingEditor` | Mediator that exposes the active `DrawingView` to figure-aware actions such as `SelectSameAction`. | +| `FigureSelectionEvent` | Event object carrying the old and new selection sets; created by `DefaultDrawingView.fireSelectionChanged()`. | +| `FigureSelectionListener` | Observer interface; every registered listener receives `selectionChanged(FigureSelectionEvent)` whenever the selection changes. | +| `AbstractAttributeEditorHandler` | Observer that reacts to selection changes by refreshing attribute editor panels (color, stroke, font, etc.). | +| `SelectionComponentDisplayer` | Observer that shows or hides toolbar UI components depending on whether figures are selected. | +| `SelectionComponentRepainter` | Observer that repaints toolbar UI components to reflect the attributes of the new selection. | + +**Total: 16 domain classes** identified by dynamic analysis. + +--- + +# Chapter: AnalysisLab1 — Packages Visited After Concept Location + +> **Feature analysed:** Automatic Selection (Select All / Deselect All / Select Same) in JHotDraw 7.6. +> **Change request used for the impact analysis:** *After Select All / Deselect All / Select Same, show a "N figure(s) selected" message in a status bar at the bottom of the Draw window, and keep it in sync with every later selection change.* + +--- + +## Table 1 — Packages, classes visited, and what each package contributes + +| Package name | # of classes visited | Comments | +|---|---:|---| +| `org.jhotdraw.action.edit` (module `jhotdraw-actions`) | 3 | App-layer Edit-menu controllers. Hosts `SelectAllAction`, `ClearSelectionAction` and their abstract base `AbstractSelectionAction`. **What I learned:** these actions are deliberately component-agnostic — they don't know about figures, they just delegate to whatever focused `EditableComponent` exists. **Contribution to the feature:** they are the controller entry points for two of the three selection scenarios (Select All, Deselect All) and resolve the focused component before forwarding the call. | +| `org.jhotdraw.api.gui` (module `jhotdraw-api`) | 1 | Defines the `EditableComponent` contract (`selectAll`, `clearSelection`, `SELECTION_EMPTY_PROPERTY`). **What I learned:** this single interface is the seam that lets the same Edit-menu actions work for the drawing canvas *and* for plain Swing text components. **Contribution to the feature:** it is the contract through which `SelectAllAction` and `ClearSelectionAction` reach the drawing view without depending on the drawing module. | +| `org.jhotdraw.draw` (module `jhotdraw-core`) | 2 | Core drawing framework. Visited classes: `DrawingEditor` and `DrawingView`. **What I learned:** `DrawingEditor` is a Mediator that exposes the active `DrawingView`, and `DrawingView` is the single source of truth for the current selection (it owns `selectedFigures`, `getSelectionCount()` and fires `FigureSelectionEvent`). **Contribution to the feature:** every selection change — whether triggered by a menu action, a toolbar button, or the mouse — ends up modifying `DrawingView` state and being broadcast from here. | +| `org.jhotdraw.draw.action` (module `jhotdraw-core`) | 2 | Drawing-aware action layer. Visited: `SelectSameAction` and its abstract base `AbstractSelectedAction`. **What I learned:** unlike the app-layer actions, these actions know about `DrawingEditor`, `DrawingView` and `Figure`, and `AbstractSelectedAction` already listens for `FigureSelectionEvent` to keep its enabled state in sync. **Contribution to the feature:** `SelectSameAction` is the third controller entry point; it inspects the currently selected figures and adds every figure of the same class to the selection. | +| `org.jhotdraw.draw.figure` (module `jhotdraw-core`) | 1 | Domain entity package containing `Figure`. **What I learned:** `SelectSameAction` only uses `Figure.getClass()` and `Figure.isSelectable()` — i.e. structural information, not behaviour. **Contribution to the feature:** figures are the things being counted/selected, but the feature does not require any new behaviour from `Figure` itself. | +| `org.jhotdraw.app` (module `jhotdraw-app`) | 3 | Application shell. Visited: `DefaultApplicationModel`, `DefaultMenuBuilder`, `AbstractView`. **What I learned:** action wiring is centralised — the model registers actions by ID, the menu builder looks them up by ID, and concrete views extend `AbstractView`. **Contribution to the feature:** this is what plugs the Edit-menu actions into every JHotDraw application without each sample having to wire them manually. | +| `org.jhotdraw.gui.action` (module `jhotdraw-gui`) | 1 | Visited: `ButtonFactory`. **What I learned:** the SVG-style toolbar with the *Select Same* button is assembled here, separate from the actions themselves. **Contribution to the feature:** it is the reason the *Select Same* button shows up on the toolbar of several samples without their panel code knowing about it. | +| `org.jhotdraw.samples.draw` (module `jhotdraw-samples-misc`) | 2 | Visited: `DrawingPanel` and `DrawView`. **What I learned:** `DrawingPanel` is just a layout container for the tools/attributes side panel, while `DrawView` is the `AbstractView` subclass that actually owns the scroll pane and the `DrawingView`. **Contribution to the feature:** `DrawView` is the natural place to host any per-window UI that reacts to selection (e.g. the status bar in the change request) because it has direct access to the `DrawingView` and its `FigureSelectionListener` registration. | + +**Total packages visited: 8** +**Total classes visited: 15** (3 seeded by concept location + 12 inspected during the marking algorithm). + +--- + +## What the table tells me overall + +- The **automatic selection** feature is spread across **two clean layers**: a generic app-layer (`jhotdraw-actions` + `jhotdraw-api`) for menu-driven actions, and a drawing-aware layer (`jhotdraw-core` + `jhotdraw-gui`) for figure-aware actions. The split is what allows the same Edit menu to work for both text components and drawing canvases. +- The drawing core is wired with the **Observer pattern** (`FigureSelectionEvent` / `FigureSelectionListener` on `DrawingView`). This is why most packages I visited ended up *UNCHANGED* during the impact analysis — the API needed to react to selection changes is already exposed. +- The **only place that actually needs new code** for the change request lives in the sample (`org.jhotdraw.samples.draw.DrawView`), confirming that JHotDraw's separation between framework and sample applications works as intended. diff --git a/concept-location-notes.html b/concept-location-notes.html new file mode 100644 index 000000000..dc6c85ce9 --- /dev/null +++ b/concept-location-notes.html @@ -0,0 +1,664 @@ + +concept-location-notes.md + + + + + + + + + + + + +
+

Concept Location Notes — Automatic Selection Feature (JHotDraw)

+

Change Request: Automatic Selection +Technique: Dynamic Program Analysis using the IDE Debugger +Sample Application: org.jhotdraw.samples.draw.Main

+
+

Scenario A — Select All

+

Trigger

+

Edit → Select All (menu item)

+

Breakpoints

+ + + + + + + + + + + + + + + + + + + + + + + + + +
FileLineMethod
jhotdraw-actions/.../SelectAllAction.java76actionPerformed()
jhotdraw-core/.../DefaultDrawingView.java844selectAll()
jhotdraw-core/.../DefaultDrawingView.java1062fireSelectionChanged()
+

Debugger Steps

+
    +
  1. F5 to launch the Draw sample.
    2. +
  2. Draw a few figures on the canvas.
    3. +
  3. Click Edit → Select All → debugger stops in SelectAllAction.actionPerformed().
    4. +
  4. F10 to step over to the line ((EditableComponent) c).selectAll();.
    5. +
  5. F11 to step into DefaultDrawingView.selectAll().
    6. +
  6. F10 through the loop — observe selectedFigures in the Variables panel growing as each Figure from drawing.getChildren() is added (only those with isSelectable() == true).
    7. +
  7. F11 at fireSelectionChanged(...) to enter event dispatch.
    8. +
  8. Inspect Call Stack panel.
    9. +
+

Observed Call Chain

+
SelectAllAction.actionPerformed()
+  └─► DefaultDrawingView.selectAll()
+        - selectedFigures.clear()
+        - for each Figure in drawing.getChildren():
+              if figure.isSelectable() → selectedFigures.add(figure)
+        - invalidateHandles()
+        - fireSelectionChanged(old, new)
+              └─► FigureSelectionEvent → all FigureSelectionListeners
+        - repaint()
+
+
+

Scenario B — Deselect All (Clear Selection)

+

Trigger

+

Edit → Clear Selection (menu item)

+

Breakpoints

+ + + + + + + + + + + + + + + + + + + + + + + + + +
FileLineMethod
jhotdraw-actions/.../ClearSelectionAction.java75actionPerformed()
jhotdraw-core/.../DefaultDrawingView.java862clearSelection()
jhotdraw-core/.../DefaultDrawingView.java1062fireSelectionChanged()
+

Debugger Steps

+
    +
  1. F5 to launch.
    2. +
  2. Draw figures and click one to select it (so the action is enabled).
    3. +
  3. Click Edit → Clear Selection → debugger stops in ClearSelectionAction.actionPerformed().
    4. +
  4. F11 at ((EditableComponent) c).clearSelection(); → enters DefaultDrawingView.clearSelection().
    5. +
  5. Watch selectedFigures in Variables panel — it shrinks to an empty set.
    6. +
  6. F11 at fireSelectionChanged(...) → enters event dispatch.
    7. +
  7. Inspect Call Stack panel.
    8. +
+

Observed Call Chain

+
ClearSelectionAction.actionPerformed()
+  └─► DefaultDrawingView.clearSelection()
+        - if selectedFigures not empty:
+            - oldSelection = copy of selectedFigures
+            - selectedFigures.clear()
+            - invalidateHandles()
+            - fireSelectionChanged(old, empty)
+                  └─► FigureSelectionEvent → all FigureSelectionListeners
+
+
+

Scenario C — Select Same

+

Trigger

+

Toolbar button "Select Same" (requires at least one figure already selected)

+

Breakpoints

+ + + + + + + + + + + + + + + + + + + + + + + + + +
FileLineMethod
jhotdraw-core/.../action/SelectSameAction.java37actionPerformed()
jhotdraw-core/.../action/SelectSameAction.java41selectSame()
jhotdraw-core/.../DefaultDrawingView.java740addToSelection(Figure)
+

Debugger Steps

+
    +
  1. F5 to launch.
    2. +
  2. Draw, e.g., two rectangles and one ellipse.
    3. +
  3. Click on one rectangle to select it (the Select Same toolbar button becomes enabled).
    4. +
  4. Click the Select Same button → debugger stops in SelectSameAction.actionPerformed().
    5. +
  5. F11 into selectSame().
    6. +
  6. Step over the first for loop — observe selectedClasses now contains the class RectangleFigure.
    7. +
  7. Step into the second loop — at getView().addToSelection(f) press F11 → enters DefaultDrawingView.addToSelection().
    8. +
  8. Watch selectedFigures grow as each rectangle is added; the ellipse is skipped because its class is not in selectedClasses.
    9. +
  9. Each call to addToSelection triggers fireSelectionChanged() — record the call stack.
    10. +
+

Observed Call Chain

+
SelectSameAction.actionPerformed()
+  └─► SelectSameAction.selectSame()
+        - selectedClasses ← classes of getView().getSelectedFigures()
+        - for each Figure f in getDrawing().getChildren():
+              if selectedClasses.contains(f.getClass())
+                  └─► DefaultDrawingView.addToSelection(f)
+                        - selectedFigures.add(figure)
+                        - figure.createHandles(detailLevel)
+                        - fireSelectionChanged(old, new)
+                              └─► FigureSelectionEvent → all FigureSelectionListeners
+
+
+

Consolidated Initial Set of Classes — Concept Location Result

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Domain ClassResponsibility
SelectAllActionController entry point for the "Select All" menu item; resolves the focused EditableComponent and invokes its selectAll() method.
ClearSelectionActionController entry point for the "Clear Selection" menu item; resolves the focused EditableComponent and invokes its clearSelection() method.
SelectSameActionController entry point for the "Select Same" toolbar action; collects the classes of currently selected figures and adds every figure of those classes to the selection.
AbstractSelectionActionAbstract base class for SelectAllAction and ClearSelectionAction; manages action enablement based on the target component's selection state.
AbstractSelectedActionAbstract base class for SelectSameAction; listens to DrawingEditor for the active view and enables the action only when at least one figure is selected.
EditableComponentContract interface declaring selectAll() and clearSelection(); implemented by DefaultDrawingView to make the drawing canvas behave as an editable component.
DrawingViewContract interface declaring the full selection API: addToSelection(), removeFromSelection(), selectAll(), clearSelection(), getSelectedFigures().
DefaultDrawingViewCore domain class that owns the selectedFigures set and implements selectAll(), clearSelection(), and addToSelection(); fires FigureSelectionEvent after every change.
DrawingDomain model that holds all figures; provides getChildren() which is iterated by selectAll() and selectSame().
FigureDomain entity representing each shape; queried via isSelectable() (Select All) and getClass() (Select Same) to decide whether it joins the selection.
DrawingEditorMediator that exposes the active DrawingView to figure-aware actions such as SelectSameAction.
FigureSelectionEventEvent object carrying the old and new selection sets; created by DefaultDrawingView.fireSelectionChanged().
FigureSelectionListenerObserver interface; every registered listener receives selectionChanged(FigureSelectionEvent) whenever the selection changes.
AbstractAttributeEditorHandlerObserver that reacts to selection changes by refreshing attribute editor panels (color, stroke, font, etc.).
SelectionComponentDisplayerObserver that shows or hides toolbar UI components depending on whether figures are selected.
SelectionComponentRepainterObserver that repaints toolbar UI components to reflect the attributes of the new selection.
+ +
+ + + \ No newline at end of file diff --git a/concept-location-notes.md b/concept-location-notes.md new file mode 100644 index 000000000..77beb537b --- /dev/null +++ b/concept-location-notes.md @@ -0,0 +1,139 @@ +# Concept Location Notes — Automatic Selection Feature (JHotDraw) + +**Change Request:** Automatic Selection +**Technique:** Dynamic Program Analysis using the IDE Debugger +**Sample Application:** `org.jhotdraw.samples.draw.Main` + +--- + +## Scenario A — Select All + +### Trigger +Edit → Select All (menu item) + +### Breakpoints +| File | Line | Method | +|------|------|--------| +| `jhotdraw-actions/.../SelectAllAction.java` | 76 | `actionPerformed()` | +| `jhotdraw-core/.../DefaultDrawingView.java` | 844 | `selectAll()` | +| `jhotdraw-core/.../DefaultDrawingView.java` | 1062 | `fireSelectionChanged()` | + +### Debugger Steps +1. F5 to launch the Draw sample. +2. Draw a few figures on the canvas. +3. Click **Edit → Select All** → debugger stops in `SelectAllAction.actionPerformed()`. +4. F10 to step over to the line `((EditableComponent) c).selectAll();`. +5. F11 to step into `DefaultDrawingView.selectAll()`. +6. F10 through the loop — observe `selectedFigures` in the Variables panel growing as each `Figure` from `drawing.getChildren()` is added (only those with `isSelectable() == true`). +7. F11 at `fireSelectionChanged(...)` to enter event dispatch. +8. Inspect Call Stack panel. + +### Observed Call Chain +``` +SelectAllAction.actionPerformed() + └─► DefaultDrawingView.selectAll() + - selectedFigures.clear() + - for each Figure in drawing.getChildren(): + if figure.isSelectable() → selectedFigures.add(figure) + - invalidateHandles() + - fireSelectionChanged(old, new) + └─► FigureSelectionEvent → all FigureSelectionListeners + - repaint() +``` + +--- + +## Scenario B — Deselect All (Clear Selection) + +### Trigger +Edit → Clear Selection (menu item) + +### Breakpoints +| File | Line | Method | +|------|------|--------| +| `jhotdraw-actions/.../ClearSelectionAction.java` | 75 | `actionPerformed()` | +| `jhotdraw-core/.../DefaultDrawingView.java` | 862 | `clearSelection()` | +| `jhotdraw-core/.../DefaultDrawingView.java` | 1062 | `fireSelectionChanged()` | + +### Debugger Steps +1. F5 to launch. +2. Draw figures and click one to select it (so the action is enabled). +3. Click **Edit → Clear Selection** → debugger stops in `ClearSelectionAction.actionPerformed()`. +4. F11 at `((EditableComponent) c).clearSelection();` → enters `DefaultDrawingView.clearSelection()`. +5. Watch `selectedFigures` in Variables panel — it shrinks to an empty set. +6. F11 at `fireSelectionChanged(...)` → enters event dispatch. +7. Inspect Call Stack panel. + +### Observed Call Chain +``` +ClearSelectionAction.actionPerformed() + └─► DefaultDrawingView.clearSelection() + - if selectedFigures not empty: + - oldSelection = copy of selectedFigures + - selectedFigures.clear() + - invalidateHandles() + - fireSelectionChanged(old, empty) + └─► FigureSelectionEvent → all FigureSelectionListeners +``` + +--- + +## Scenario C — Select Same + +### Trigger +Toolbar button "Select Same" (requires at least one figure already selected) + +### Breakpoints +| File | Line | Method | +|------|------|--------| +| `jhotdraw-core/.../action/SelectSameAction.java` | 37 | `actionPerformed()` | +| `jhotdraw-core/.../action/SelectSameAction.java` | 41 | `selectSame()` | +| `jhotdraw-core/.../DefaultDrawingView.java` | 740 | `addToSelection(Figure)` | + +### Debugger Steps +1. F5 to launch. +2. Draw, e.g., two rectangles and one ellipse. +3. Click on one rectangle to select it (the Select Same toolbar button becomes enabled). +4. Click the **Select Same** button → debugger stops in `SelectSameAction.actionPerformed()`. +5. F11 into `selectSame()`. +6. Step over the first `for` loop — observe `selectedClasses` now contains the class `RectangleFigure`. +7. Step into the second loop — at `getView().addToSelection(f)` press F11 → enters `DefaultDrawingView.addToSelection()`. +8. Watch `selectedFigures` grow as each rectangle is added; the ellipse is skipped because its class is not in `selectedClasses`. +9. Each call to `addToSelection` triggers `fireSelectionChanged()` — record the call stack. + +### Observed Call Chain +``` +SelectSameAction.actionPerformed() + └─► SelectSameAction.selectSame() + - selectedClasses ← classes of getView().getSelectedFigures() + - for each Figure f in getDrawing().getChildren(): + if selectedClasses.contains(f.getClass()) + └─► DefaultDrawingView.addToSelection(f) + - selectedFigures.add(figure) + - figure.createHandles(detailLevel) + - fireSelectionChanged(old, new) + └─► FigureSelectionEvent → all FigureSelectionListeners +``` + +--- + +## Consolidated Initial Set of Classes — Concept Location Result + +| Domain Class | Responsibility | +|---|---| +| `SelectAllAction` | Controller entry point for the "Select All" menu item; resolves the focused `EditableComponent` and invokes its `selectAll()` method. | +| `ClearSelectionAction` | Controller entry point for the "Clear Selection" menu item; resolves the focused `EditableComponent` and invokes its `clearSelection()` method. | +| `SelectSameAction` | Controller entry point for the "Select Same" toolbar action; collects the classes of currently selected figures and adds every figure of those classes to the selection. | +| `AbstractSelectionAction` | Abstract base class for `SelectAllAction` and `ClearSelectionAction`; manages action enablement based on the target component's selection state. | +| `AbstractSelectedAction` | Abstract base class for `SelectSameAction`; listens to `DrawingEditor` for the active view and enables the action only when at least one figure is selected. | +| `EditableComponent` | Contract interface declaring `selectAll()` and `clearSelection()`; implemented by `DefaultDrawingView` to make the drawing canvas behave as an editable component. | +| `DrawingView` | Contract interface declaring the full selection API: `addToSelection()`, `removeFromSelection()`, `selectAll()`, `clearSelection()`, `getSelectedFigures()`. | +| `DefaultDrawingView` | Core domain class that owns the `selectedFigures` set and implements `selectAll()`, `clearSelection()`, and `addToSelection()`; fires `FigureSelectionEvent` after every change. | +| `Drawing` | Domain model that holds all figures; provides `getChildren()` which is iterated by `selectAll()` and `selectSame()`. | +| `Figure` | Domain entity representing each shape; queried via `isSelectable()` (Select All) and `getClass()` (Select Same) to decide whether it joins the selection. | +| `DrawingEditor` | Mediator that exposes the active `DrawingView` to figure-aware actions such as `SelectSameAction`. | +| `FigureSelectionEvent` | Event object carrying the old and new selection sets; created by `DefaultDrawingView.fireSelectionChanged()`. | +| `FigureSelectionListener` | Observer interface; every registered listener receives `selectionChanged(FigureSelectionEvent)` whenever the selection changes. | +| `AbstractAttributeEditorHandler` | Observer that reacts to selection changes by refreshing attribute editor panels (color, stroke, font, etc.). | +| `SelectionComponentDisplayer` | Observer that shows or hides toolbar UI components depending on whether figures are selected. | +| `SelectionComponentRepainter` | Observer that repaints toolbar UI components to reflect the attributes of the new selection. | From b90333e0f06f423cca548efbf437bf929457249d Mon Sep 17 00:00:00 2001 From: Klaus Fabo Date: Thu, 4 Jun 2026 14:36:28 +0200 Subject: [PATCH 13/18] Add Lab 1 undo/redo concept-location & impact-analysis notes; force JDK XML parser for samples-misc Co-Authored-By: Claude Opus 4.8 (1M context) --- .../jhotdraw-samples-misc/pom.xml | 19 ++ ...b1-concept-location-and-impact-analysis.md | 180 ++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 undo-redo/lab1-concept-location-and-impact-analysis.md diff --git a/jhotdraw-samples/jhotdraw-samples-misc/pom.xml b/jhotdraw-samples/jhotdraw-samples-misc/pom.xml index ca8104ee5..e3a927090 100644 --- a/jhotdraw-samples/jhotdraw-samples-misc/pom.xml +++ b/jhotdraw-samples/jhotdraw-samples-misc/pom.xml @@ -47,6 +47,25 @@ org.codehaus.mojo exec-maven-plugin 3.1.0 + + + + + javax.xml.parsers.DocumentBuilderFactory + com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl + + + javax.xml.parsers.SAXParserFactory + com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl + + + diff --git a/undo-redo/lab1-concept-location-and-impact-analysis.md b/undo-redo/lab1-concept-location-and-impact-analysis.md new file mode 100644 index 000000000..61b4aab28 --- /dev/null +++ b/undo-redo/lab1-concept-location-and-impact-analysis.md @@ -0,0 +1,180 @@ +# Lab 1 — Concept Location & Impact Analysis (Undo/Redo) + +> **Project:** JHotDraw · **Sample app:** `org.jhotdraw.samples.draw.Main` +> **Feature under study:** the Undo/Redo concept +> **Worked change request (CR):** *Add an "Undo All / Redo All" command* + +This document has two parts: + +1. **[Part A — Concept Location](#part-a--concept-location)** — find the code that implements Undo/Redo by debugging the running app. +2. **[Part B — Impact Analysis](#part-b--impact-analysis)** — starting from the located class, run the change-propagation algorithm (Figure 7.9) to estimate which classes a change would touch. + +--- + +## Part A — Concept Location + +### A.0 Lab brief + +**Introduction.** Dynamic program analysis is the analysis of software performed by *executing* the program on a real or virtual processor. To be effective, the target program must be run with sufficient test inputs to produce interesting behavior. Coverage measures help ensure that an adequate slice of the program's possible behaviors has been observed. + +**Objectives.** +- Apply the IDE debugger to locate feature concepts **at runtime**. +- Produce the **initial set of classes** that results from concept location. + +**Classwork.** Use the IDE debugger to localize the classes involved in the Change Request. Features usually start from **controller** classes. If the feature spans many classes, localize only the **domain** classes tied to the domain concepts. + +> **This submission:** Feature = **Undo/Redo**; the gesture traced is **Edit → Undo** in `org.jhotdraw.samples.draw.Main`. The controller entry point is `UndoAction.actionPerformed` (see A.2–A.5). The resulting initial set of classes is the **Portfolio deliverable in [A.6](#a6-portfolio-deliverable--initial-set-of-classes)**. + +### A.1 Setup for this feature + +1. Run `org.jhotdraw.samples.draw.Main` in **Debug** mode and let the app open fully. +2. In the running app, **draw a figure** (so there is something to undo). +3. Trigger **Edit → Undo**. + +> **Not in `Main.java`.** `Main.main()` runs once at startup and never again, so a breakpoint there tells you nothing about undo/redo. Let `Main` run normally and put the breakpoint in the class that *handles the Undo gesture*. + +### A.2 The key finding + +There are **two `UndoAction` classes**, connected by a **delegation pattern**. Discovering this link is the main goal of the debugging session. + +| Role | Location | What it does | +|------|----------|--------------| +| **Menu controller** | `jhotdraw-actions/.../org/jhotdraw/action/edit/UndoAction.java` | Does **not** perform the undo. Looks up the real action from the active view's `ActionMap` under the ID `"edit.undo"` and delegates: `realUndoAction.actionPerformed(e)`. | +| **Real action** | inner `UndoAction` of `jhotdraw-utils/.../org/jhotdraw/undo/UndoRedoManager.java` | Registered by the view via `getActionMap().put(UndoAction.ID, undo.getUndoAction())` in `DrawView.java`. Calls `UndoRedoManager.undo()`. | + +(Do the same for **Redo** in `RedoAction.java`.) + +### A.3 Where to set breakpoints + +**Primary breakpoint** — first line inside `actionPerformed` of the *menu* controller +`jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/UndoAction.java`: + +```java +@Override +public void actionPerformed(ActionEvent e) { + Action realUndoAction = getRealUndoAction(); // ← put the breakpoint here + if (realUndoAction != null && realUndoAction != this) { + realUndoAction.actionPerformed(e); + } +} +``` + +**Optional breakpoints** — to also see how edits get *recorded* (the other half of the concept): + +- `addEdit(...)` in `UndoRedoManager` (`jhotdraw-utils/.../org/jhotdraw/undo/UndoRedoManager.java`) +- `fireUndoableEditHappened(...)` in `AbstractFigure` + +### A.4 How to step through it + +1. Run `Main` in Debug mode; let the app open. +2. Draw a figure. +3. Click **Edit → Undo** → execution stops at the primary breakpoint. +4. **Step Into (F7)**: `getRealUndoAction()` returns the action stored in the view; stepping into `realUndoAction.actionPerformed(e)` jumps into `UndoRedoManager`'s inner `UndoAction`, then into `UndoRedoManager.undo()`. The **call-stack panel** records each class as you go. + +### A.5 Execution paths + +**Recording (an edit is created and stored):** + +``` +figure changes + → AbstractFigure.fireUndoableEditHappened + → Drawing.fireUndoableEditHappened + → UndoRedoManager.undoableEditHappened (UndoableEditListener) + → UndoRedoManager.addEdit +``` + +**Undo trigger (a stored edit is reversed):** + +``` +menu UndoAction.actionPerformed + → UndoRedoManager$UndoAction.actionPerformed (delegated real action) + → UndoRedoManager.undo() + → UndoableEdit.undo() (reverses the figure change) +``` + +### A.6 Portfolio deliverable — Initial set of classes + +> **Portfolio Work.** Per the lab brief, the result of concept location is the initial set of classes in the required `Domain Class | Responsibility` table format below. + +| Domain Class | Responsibility | +|--------------|----------------| +| **`UndoAction` / `RedoAction`** (`org.jhotdraw.action.edit`) | Menu/toolbar controller for Undo/Redo; a thin wrapper that **delegates** to the real action registered in the active view. | +| **`AbstractViewAction`** | Parent of the menu actions; resolves the currently active view so the command targets the right document. | +| **`View` / `DrawView`** (sample) | Owns a per-view `UndoRedoManager`, listens to its drawing for edits, and registers the real undo/redo actions in its `ActionMap`. | +| **`UndoRedoManager`** (`org.jhotdraw.undo`) | **Core of the concept.** Extends Swing's `UndoManager`, holds the edit stack, exposes the real undo/redo actions, and performs `undo()`/`redo()`. | +| **`UndoRedoManager.UndoAction` / `.RedoAction`** (inner) | The **real** actions stored in the view; invoke `manager.undo()` / `manager.redo()` and keep their enabled state in sync. | +| **`Drawing` / `DefaultDrawing`** | The model; fires `UndoableEditEvent`s and is the source the manager subscribes to as an `UndoableEditListener`. | +| **`AbstractFigure`** (and composite figures) | Generate undoable edits via `fireUndoableEditHappened` whenever a figure is modified. | +| **`UndoableEdit` / `AbstractUndoableEdit` / `CompositeEdit`** | The edit objects that encapsulate a change and know how to reverse (`undo`) and reapply (`redo`) it. | + +> **Trimming tip:** If your CR scopes undo/redo to one specific operation (e.g. undoing a *move* or a *group*), you can trim this to the controller (`UndoAction`), the `UndoRedoManager`, and the particular `UndoableEdit` subclass that operation creates. + +**Rule of thumb:** the breakpoint goes on the **controller that handles the gesture you trigger**, not on the launcher. For undo/redo that is `UndoAction.actionPerformed`. + +--- + +## Part B — Impact Analysis + +This is the step *after* concept location. Start from the class you located (`UndoRedoManager` / `UndoAction`) and follow the **change-propagation algorithm of Figure 7.9** to estimate which other classes a change would touch. + +### B.1 The algorithm (plain terms) + +Build an interaction diagram of the classes that collaborate for the feature (most already come from Part A). Every class starts **BLANK**, then: + +1. Mark the located class as **CHANGED** — this is your **seed**. +2. Mark all its BLANK neighbors as **NEXT** (the frontier to examine). +3. While any class is **NEXT**, pick one, inspect it, and give it one of three marks: + - **CHANGED** — must be modified to make the change work. + - **PROPAGATES** — not modified itself, but the change passes through it (a forwarded call, a relayed signature change), so its neighbors still need checking. + - **UNCHANGED** — not modified and does not carry the change further. +4. If you marked it **CHANGED** or **PROPAGATES**, mark its BLANK neighbors as **NEXT**. If **UNCHANGED**, stop expanding from it. +5. Repeat until no **NEXT** classes remain. + +> **Estimated Impact Set** = every class marked **CHANGED** or **PROPAGATES**. UNCHANGED classes were visited but excluded. + +You find neighbors using both kinds of analysis the objective names: + +- **Static** — IDE "Find Usages" / "Call Hierarchy" on each class. +- **Dynamic** — the debugger call stack from Part A (`UndoAction → UndoRedoManager → undo()`). + +> **Caveat — the marks depend entirely on the CR.** "Fix a bug in undo" propagates very differently from "add Undo All." The worked pass below uses **CR = "Add Undo All / Redo All."** Substitute your own CR's judgments. + +### B.2 Worked propagation pass — 7 steps + +> Legend: 🟧 **CHANGED** · 🟦 **UNCHANGED** · 🟪 **NEXT** (frontier) · ⬜ **BLANK** (never reached) + +| Step | Action | Result | +|:----:|--------|--------| +| **0** | Initialize | All classes **BLANK**. | +| **1** | **Seed.** `UndoRedoManager` was located in Part A; it needs new `undoAll()`/`redoAll()` methods. | `UndoRedoManager` → 🟧 **CHANGED**. Neighbors → 🟪 **NEXT**: `DrawView`, `UndoableEdit`, `DefaultDrawing`. | +| **2** | Examine **`DrawView`** (NEXT). It owns the manager and registers its actions into the `ActionMap` (≈ lines 131–132). New manager actions ⇒ must register them. | `DrawView` → 🟧 **CHANGED**. Mark BLANK neighbor `UndoAction` → 🟪 **NEXT**. | +| **3** | Examine **`UndoableEdit`** (NEXT). The `javax.swing.undo` interface from the JDK; the existing `undo()` contract is reused. | `UndoableEdit` → 🟦 **UNCHANGED**. No expansion. | +| **4** | Examine **`DefaultDrawing`** (NEXT). Fires `UndoableEditEvent`s via the standard `UndoableEditListener`; that mechanism is unchanged for Undo All. | `DefaultDrawing` → 🟦 **UNCHANGED**. No expansion → `AbstractFigure` stays ⬜ BLANK. | +| **5** | Examine **`UndoAction`** (NEXT). Delegates to `getActiveView().getActionMap().get(ID)`. The delegation pattern is unchanged — a new `UndoAllAction` would be a *separate* class following the same pattern, not a modification here. | `UndoAction` → 🟦 **UNCHANGED**. No expansion → `AbstractViewAction` stays ⬜ BLANK. | +| **6** | No 🟪 NEXT classes remain. | **Algorithm terminates.** | + +**Estimated Impact Set = { `UndoRedoManager`, `DrawView` }** (the two 🟧 coral classes). + +- 🟦 **Visited but unaffected:** `UndoAction`, `DefaultDrawing`, `UndoableEdit`. +- ⬜ **Never reached:** `AbstractFigure`, `AbstractViewAction`, and the rest — the change is well-encapsulated and does not ripple to them. + +### B.3 Table 1 — packages visited + +Keep the **Comments** column honest about what you *learned* by visiting each package — that is explicitly what the portfolio asks for. Re-decide every CHANGED/PROPAGATES/UNCHANGED mark against your real CR; the structure stays the same, but which packages land in the impact set will shift. + +| Package | # classes visited | Mark | Comments | +|---------|:-----------------:|------|----------| +| `org.jhotdraw.action.edit` | 2 | CHANGED* | The Undo/Redo menu controllers — the feature's command surface. New commands are added or existing ones extended. *(In the worked Undo-All pass `UndoAction` itself stayed UNCHANGED because a parallel `UndoAllAction` is a new sibling class.)* | +| `org.jhotdraw.action` | 1 | PROPAGATES | `AbstractViewAction`, base for view-scoped actions. Not modified, but every undo/redo action inherits its active-view resolution. | +| `org.jhotdraw.undo` | 1–2 | CHANGED | `UndoRedoManager` (and `CompositeEdit`). Core of the feature; the actual undo/redo logic and edit stack live here. | +| `org.jhotdraw.samples.draw` | 2 | CHANGED | `DrawView`, `DrawApplicationModel`. The view owns the per-document manager and registers the real actions; the model builds the menu. | +| `org.jhotdraw.draw` | 2–3 | UNCHANGED | `Drawing`, `DefaultDrawing`, `DefaultDrawingView`. The model fires `UndoableEditEvent`s — the *source* the manager listens to, not something the change modifies. | +| `org.jhotdraw.draw.figure` | 1+ | UNCHANGED | `AbstractFigure` and figure subclasses. They generate edits when modified; visited to confirm the edit-creation path is unaffected. | +| `org.jhotdraw.app` / `org.jhotdraw.api.app` | 2–3 | PROPAGATES / UNCHANGED | Application/View framework (`SDIApplication`, `View`). Provides the wiring that hands the action its active view. | +| `javax.swing.undo` (external) | 3–4 | UNCHANGED | `UndoManager`, `UndoableEdit`, `AbstractUndoableEdit`, `CompoundEdit`. JDK library that `UndoRedoManager` extends; reused, never modified. | + +### B.4 Takeaways + +1. **A small impact set is a valid — and good — result.** It means the feature has **low coupling**; for the Undo-All CR only `UndoRedoManager` and `DrawView` need changing. +2. **The marks shift with the CR.** If a CR required modifying the `UndoableEdit` interface itself, it would flip to **CHANGED** and propagate outward through `DefaultDrawing` and `AbstractFigure`, giving a much larger impact set. The **algorithm is identical** either way — what changes is your judgment at each NEXT class about whether it needs modification. +3. **For the portfolio:** the two CHANGED classes plus the three UNCHANGED ones are the packages you "visited"; the gray (never-reached) ones you did not. From 34d2fe2c4eb5df3387060e4198d6ecb3f4cc2d10 Mon Sep 17 00:00:00 2001 From: Klaus Fabo Date: Thu, 4 Jun 2026 15:12:51 +0200 Subject: [PATCH 14/18] Lab2 Smells with SonarLint and refactoring --- .../action/edit/AbstractUndoRedoAction.java | 116 ++++++++++++++++++ .../org/jhotdraw/action/edit/RedoAction.java | 88 +------------ .../org/jhotdraw/action/edit/UndoAction.java | 89 ++------------ ...b1-concept-location-and-impact-analysis.md | 1 + undo-redo/lab2-code-smells-sonarlint.md | 67 ++++++++++ 5 files changed, 197 insertions(+), 164 deletions(-) create mode 100644 jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/AbstractUndoRedoAction.java create mode 100644 undo-redo/lab2-code-smells-sonarlint.md diff --git a/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/AbstractUndoRedoAction.java b/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/AbstractUndoRedoAction.java new file mode 100644 index 000000000..4becaf341 --- /dev/null +++ b/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/AbstractUndoRedoAction.java @@ -0,0 +1,116 @@ +/* + * @(#)AbstractUndoRedoAction.java + * + * Copyright (c) 1996-2010 The authors and contributors of JHotDraw. + * You may not use, copy or modify this file, except in compliance with the + * accompanying license terms. + */ +package org.jhotdraw.action.edit; + +import java.awt.event.ActionEvent; +import java.beans.PropertyChangeEvent; +import java.beans.PropertyChangeListener; +import javax.swing.AbstractAction; +import javax.swing.Action; +import org.jhotdraw.action.AbstractViewAction; +import org.jhotdraw.api.app.Application; +import org.jhotdraw.api.app.View; +import org.jhotdraw.util.ResourceBundleUtil; + +/** + * Common behaviour for the Undo and Redo menu actions. + *

+ * These actions do not perform the undo/redo themselves. They look up the + * "real" action that the active {@link View} registered in its + * {@code ActionMap} under a given ID, and delegate to it. This class captures + * the shared delegation, listener wiring and enabled-state synchronisation; + * concrete subclasses only supply the action ID + * (see {@link UndoAction} and {@link RedoAction}). + * + * @author Werner Randelshofer + * @version $Id$ + */ +public abstract class AbstractUndoRedoAction extends AbstractViewAction { + + private static final long serialVersionUID = 1L; + + /** The ActionMap key of the real action this menu action delegates to. */ + private final String id; + private final ResourceBundleUtil labels = + ResourceBundleUtil.getBundle("org.jhotdraw.action.Labels"); + /** Keeps this action's name/enabled state in sync with the real action. */ + private final PropertyChangeListener actionPropertyListener = + this::realActionPropertyChanged; + + /** + * Creates a new instance that delegates to the action registered under + * {@code id} in the active view's {@code ActionMap}. + */ + protected AbstractUndoRedoAction(Application app, View view, String id) { + super(app, view); + this.id = id; + labels.configureAction(this, id); + } + + private void realActionPropertyChanged(PropertyChangeEvent evt) { + String name = evt.getPropertyName(); + if ((name == null && AbstractAction.NAME == null) + || (name != null && name.equals(AbstractAction.NAME))) { + putValue(AbstractAction.NAME, evt.getNewValue()); + } else if ("enabled".equals(name)) { + updateEnabledState(); + } + } + + protected void updateEnabledState() { + boolean isEnabled = false; + Action realAction = getRealAction(); + if (realAction != null && realAction != this) { + isEnabled = realAction.isEnabled(); + } + setEnabled(isEnabled); + } + + @Override + protected void updateView(View oldValue, View newValue) { + super.updateView(oldValue, newValue); + if (newValue != null + && newValue.getActionMap().get(id) != null + && newValue.getActionMap().get(id) != this) { + putValue(AbstractAction.NAME, + newValue.getActionMap().get(id).getValue(AbstractAction.NAME)); + updateEnabledState(); + } + } + + @Override + protected void installViewListeners(View p) { + super.installViewListeners(p); + Action actionInView = p.getActionMap().get(id); + if (actionInView != null && actionInView != this) { + actionInView.addPropertyChangeListener(actionPropertyListener); + } + } + + @Override + protected void uninstallViewListeners(View p) { + super.uninstallViewListeners(p); + Action actionInView = p.getActionMap().get(id); + if (actionInView != null && actionInView != this) { + actionInView.removePropertyChangeListener(actionPropertyListener); + } + } + + @Override + public void actionPerformed(ActionEvent e) { + Action realAction = getRealAction(); + if (realAction != null && realAction != this) { + realAction.actionPerformed(e); + } + } + + /** Returns the real action registered in the active view, or null. */ + protected Action getRealAction() { + return (getActiveView() == null) ? null : getActiveView().getActionMap().get(id); + } +} diff --git a/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/RedoAction.java b/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/RedoAction.java index 794ab08c8..429a61ffd 100644 --- a/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/RedoAction.java +++ b/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/RedoAction.java @@ -7,110 +7,34 @@ */ package org.jhotdraw.action.edit; -import java.awt.event.*; -import java.beans.*; -import javax.swing.*; -import org.jhotdraw.action.AbstractViewAction; import org.jhotdraw.api.app.Application; import org.jhotdraw.api.app.View; -import org.jhotdraw.util.*; /** * Redoes the last user action on the active view. *

- * This action requires that the View returns a project - * specific redo action when invoking getActionMap("redo") on a View. + * This action requires that the View returns a project specific redo action + * when invoking {@code getActionMap().get("edit.redo")} on a View. *

- * This action is called when the user selects the Redo item in the Edit - * menu. The menu item is automatically created by the application. + * This action is called when the user selects the Redo item in the Edit menu. + * The menu item is automatically created by the application. *

* If you want this behavior in your application, you have to create an action * with this ID and put it in your {@code ApplicationModel} in method * {@link org.jhotdraw.app.ApplicationModel#initApplication}. * - * * @author Werner Randelshofer * @version $Id$ */ -public class RedoAction extends AbstractViewAction { +public class RedoAction extends AbstractUndoRedoAction { private static final long serialVersionUID = 1L; public static final String ID = "edit.redo"; - private ResourceBundleUtil labels = ResourceBundleUtil.getBundle("org.jhotdraw.action.Labels"); - private PropertyChangeListener redoActionPropertyListener = new PropertyChangeListener() { - @Override - public void propertyChange(PropertyChangeEvent evt) { - String name = evt.getPropertyName(); - if ((name == null && AbstractAction.NAME == null) || (name != null && name.equals(AbstractAction.NAME))) { - putValue(AbstractAction.NAME, evt.getNewValue()); - } else if ("enabled".equals(name)) { - updateEnabledState(); - } - } - }; /** * Creates a new instance. */ public RedoAction(Application app, View view) { - super(app, view); - labels.configureAction(this, ID); - } - - protected void updateEnabledState() { - boolean isEnabled = false; - Action realRedoAction = getRealRedoAction(); - if (realRedoAction != null && realRedoAction != this) { - isEnabled = realRedoAction.isEnabled(); - } - setEnabled(isEnabled); - } - - @Override - protected void updateView(View oldValue, View newValue) { - super.updateView(oldValue, newValue); - if (newValue != null - && newValue.getActionMap().get(ID) != null - && newValue.getActionMap().get(ID) != this) { - putValue(AbstractAction.NAME, newValue.getActionMap().get(ID). - getValue(AbstractAction.NAME)); - updateEnabledState(); - } - } - - /** - * Installs listeners on the view object. - */ - @Override - protected void installViewListeners(View p) { - super.installViewListeners(p); - Action redoActionInView = p.getActionMap().get(ID); - if (redoActionInView != null && redoActionInView != this) { - redoActionInView.addPropertyChangeListener(redoActionPropertyListener); - } - } - - /** - * Installs listeners on the view object. - */ - @Override - protected void uninstallViewListeners(View p) { - super.uninstallViewListeners(p); - Action redoActionInView = p.getActionMap().get(ID); - if (redoActionInView != null && redoActionInView != this) { - redoActionInView.removePropertyChangeListener(redoActionPropertyListener); - } - } - - @Override - public void actionPerformed(ActionEvent e) { - Action realAction = getRealRedoAction(); - if (realAction != null && realAction != this) { - realAction.actionPerformed(e); - } - } - - private Action getRealRedoAction() { - return (getActiveView() == null) ? null : getActiveView().getActionMap().get(ID); + super(app, view, ID); } } diff --git a/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/UndoAction.java b/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/UndoAction.java index 74c4f2cea..4f9070ad9 100644 --- a/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/UndoAction.java +++ b/jhotdraw-actions/src/main/java/org/jhotdraw/action/edit/UndoAction.java @@ -7,22 +7,17 @@ */ package org.jhotdraw.action.edit; -import java.awt.event.*; -import java.beans.*; -import javax.swing.*; -import org.jhotdraw.action.AbstractViewAction; import org.jhotdraw.api.app.Application; import org.jhotdraw.api.app.View; -import org.jhotdraw.util.*; /** - * Undoes the last user action. + * Undoes the last user action on the active view. *

- * This action requires that the View returns a project - * specific undo action when invoking getActionMap("redo") on a View. + * This action requires that the View returns a project specific undo action + * when invoking {@code getActionMap().get("edit.undo")} on a View. *

- * This action is called when the user selects the Undo item in the Edit - * menu. The menu item is automatically created by the application. + * This action is called when the user selects the Undo item in the Edit menu. + * The menu item is automatically created by the application. *

* If you want this behavior in your application, you have to create an action * with this ID and put it in your {@code ApplicationModel} in method @@ -31,85 +26,15 @@ * @author Werner Randelshofer * @version $Id$ */ -public class UndoAction extends AbstractViewAction { +public class UndoAction extends AbstractUndoRedoAction { private static final long serialVersionUID = 1L; public static final String ID = "edit.undo"; - private ResourceBundleUtil labels = ResourceBundleUtil.getBundle("org.jhotdraw.action.Labels"); - private PropertyChangeListener redoActionPropertyListener = new PropertyChangeListener() { - @Override - public void propertyChange(PropertyChangeEvent evt) { - String name = evt.getPropertyName(); - if ((name == null && AbstractAction.NAME == null) || (name != null && name.equals(AbstractAction.NAME))) { - putValue(AbstractAction.NAME, evt.getNewValue()); - } else if ("enabled".equals(name)) { - updateEnabledState(); - } - } - }; /** * Creates a new instance. */ public UndoAction(Application app, View view) { - super(app, view); - labels.configureAction(this, ID); - } - - protected void updateEnabledState() { - boolean isEnabled = false; - Action realAction = getRealUndoAction(); - if (realAction != null && realAction != this) { - isEnabled = realAction.isEnabled(); - } - setEnabled(isEnabled); - } - - @Override - protected void updateView(View oldValue, View newValue) { - super.updateView(oldValue, newValue); - if (newValue != null - && newValue.getActionMap().get(ID) != null - && newValue.getActionMap().get(ID) != this) { - putValue(AbstractAction.NAME, newValue.getActionMap().get(ID). - getValue(AbstractAction.NAME)); - updateEnabledState(); - } - } - - /** - * Installs listeners on the view object. - */ - @Override - protected void installViewListeners(View p) { - super.installViewListeners(p); - Action undoActionInView = p.getActionMap().get(ID); - if (undoActionInView != null && undoActionInView != this) { - undoActionInView.addPropertyChangeListener(redoActionPropertyListener); - } - } - - /** - * Installs listeners on the view object. - */ - @Override - protected void uninstallViewListeners(View p) { - super.uninstallViewListeners(p); - Action undoActionInView = p.getActionMap().get(ID); - if (undoActionInView != null && undoActionInView != this) { - undoActionInView.removePropertyChangeListener(redoActionPropertyListener); - } - } - - @Override - public void actionPerformed(ActionEvent e) { - Action realUndoAction = getRealUndoAction(); - if (realUndoAction != null && realUndoAction != this) { - realUndoAction.actionPerformed(e); - } - } - - private Action getRealUndoAction() { - return (getActiveView() == null) ? null : getActiveView().getActionMap().get(ID); + super(app, view, ID); } } diff --git a/undo-redo/lab1-concept-location-and-impact-analysis.md b/undo-redo/lab1-concept-location-and-impact-analysis.md index 61b4aab28..470a5e15f 100644 --- a/undo-redo/lab1-concept-location-and-impact-analysis.md +++ b/undo-redo/lab1-concept-location-and-impact-analysis.md @@ -178,3 +178,4 @@ Keep the **Comments** column honest about what you *learned* by visiting each pa 1. **A small impact set is a valid — and good — result.** It means the feature has **low coupling**; for the Undo-All CR only `UndoRedoManager` and `DrawView` need changing. 2. **The marks shift with the CR.** If a CR required modifying the `UndoableEdit` interface itself, it would flip to **CHANGED** and propagate outward through `DefaultDrawing` and `AbstractFigure`, giving a much larger impact set. The **algorithm is identical** either way — what changes is your judgment at each NEXT class about whether it needs modification. 3. **For the portfolio:** the two CHANGED classes plus the three UNCHANGED ones are the packages you "visited"; the gray (never-reached) ones you did not. + diff --git a/undo-redo/lab2-code-smells-sonarlint.md b/undo-redo/lab2-code-smells-sonarlint.md new file mode 100644 index 000000000..7e8f3993c --- /dev/null +++ b/undo-redo/lab2-code-smells-sonarlint.md @@ -0,0 +1,67 @@ +# Lab 2 — Code Smells (SonarLint / SonarQube for IDE) + +> **Project:** JHotDraw · **Feature / CR:** Undo/Redo (same classes located in [Lab 1](lab1-concept-location-and-impact-analysis.md)) +> **Tool:** SonarQube for IDE (SonarLint) — VS Code extension, **Connected Mode** to SonarQube Cloud (project `domjab23`) + +## Objective + +Find code smells in JHotDraw **scoped to the change request** — the undo/redo classes located in Lab 1 — using SonarLint static analysis in the IDE, and record them in table form for the portfolio. + +## 1. How the analysis was run + +1. Installed the **SonarQube for IDE (SonarLint)** extension and bound it in **Connected Mode** to SonarQube Cloud project `domjab23`. +2. Opened each CR class from Lab 1 so SonarLint analyzes it on open: + - `jhotdraw-utils/.../org/jhotdraw/undo/UndoRedoManager.java` + - `jhotdraw-actions/.../org/jhotdraw/action/edit/UndoAction.java` + - `jhotdraw-actions/.../org/jhotdraw/action/edit/RedoAction.java` + - `jhotdraw-samples-misc/.../org/jhotdraw/samples/draw/DrawView.java` +3. Read findings from three places: + - **inline squiggles** in each file, + - the **Problems panel** (`Ctrl+Shift+M`), filtered to `sonar`, + - the **SonarLint view** — each finding shows a rule id such as `java:S2208`. + +> **Scope:** the project-wide warning count was ~592; this lab reports only the subset in the CR classes — those are the smells relevant to the undo/redo change. + +## 2. Findings (CR-scoped) + +> Severity uses SonarQube's labels. Because SonarLint is in **Connected Mode**, the active **quality profile** of `domjab23` is authoritative — confirm each rule id/severity against what your IDE actually reports. + +| # | Class (file) | Line(s) | Rule (id) | Severity | Smell — what & why | +|:-:|--------------|---------|-----------|----------|--------------------| +| 1 | `UndoRedoManager` | 10–15 | `java:S2208` Wildcard imports should not be used | Minor | 6 star-imports (`java.awt.event.*`, `java.beans.*`, `java.util.*`, `javax.swing.*`, …) hide what is actually used and risk name clashes. | +| 2 | `UndoRedoManager` | 86, 113, 191, 226 | `java:S106` Standard outputs should not be used to log | Major | Logs via `System.out`/`System.err` instead of a logger — no levels, no formatting, not configurable. | +| 3 | `UndoRedoManager` | 87 | `java:S1148` `printStackTrace()` should not be called | Minor | `e.printStackTrace()` in the `catch` writes raw traces to console. | +| 4 | `UndoRedoManager` | 142–144 | `java:S2696` Instance methods should not write to static fields | Major | `setLocale()` (instance method) reassigns the **static** `labels` field — hidden global state shared across all managers. | +| 5 | `UndoRedoManager` | 28 + `if (DEBUG)` at 190, 225 | dead conditional / unused logging guard | Minor | `private static final boolean DEBUG = false;` makes the `if (DEBUG)` blocks unreachable dead code. | +| 6 | `UndoRedoManager` | 24 | `java:S125` Commented-out code should be removed | Minor | Trailing `//javax.swing.undo.UndoManager {` left in the class declaration. | +| 7 | `UndoRedoManager` | 82–88 vs 108–114 | inconsistent exception handling | Minor | `undo()`'s catch logs to `System.err` + stack trace; `redo()`'s catch only `System.out.println`, no trace — asymmetric handling of the same kind of failure. | +| 8 | `UndoAction` & `RedoAction` | 10–16 | `java:S2208` Wildcard imports | Minor | Same star-import smell in both action classes. | +| 9 | `UndoAction` & `RedoAction` | 39–49 / 40–50 | `java:S1604` Anonymous class should be a lambda | Minor | `new PropertyChangeListener() { … }` is a single-method interface → should be a lambda (Java 8+). | +| 10 | `UndoAction` | 39 | misleading name (copy-paste smell) | Minor | The field is named **`redoActionPropertyListener`** inside **`UndoAction`** — copied from `RedoAction`; it actually listens to the *undo* action. Misleading identifier. | +| 11 | `UndoAction` | 21–22 (Javadoc) | copy-paste documentation error | Info | Doc says *"returns a project specific undo action when invoking `getActionMap("redo")`"* — wrong ID in the prose (`"redo"` instead of `"undo"`), copied from `RedoAction`. | +| 12 | `UndoAction` vs `RedoAction` | whole class | `java:S4144` / duplicated code | Major | The two classes are near-identical (same structure, listener, install/uninstall logic) — duplicated logic that could share a base class. | + +### 2a. `DrawView.java` — confirmed SonarLint output + +Transcribed verbatim from the SonarQube-for-IDE Problems panel (`DrawView.java`, **10 issues**). These are the authoritative results from the connected `domjab23` profile. + +| # | Location | Rule (id) | Message | +|:-:|----------|-----------|---------| +| 13 | Ln 64, Col 27 | `java:S1948` | Make non-static `"editor"` transient or serializable. *(non-serializable field in a `Serializable` class)* | +| 14 | Ln 78, Col 44 | `java:S1604` | Make this anonymous inner class a lambda. | +| 15 | Ln 98, Col 50 | `java:S3252` | Use static access with `"javax.swing.ScrollPaneConstants"` for `"LOWER_LEFT_CORNER"`. | +| 16 | Ln 160, Col 17 | `java:S1141` | Extract this nested `try` block into a separate method. *(+1 location)* | +| 17 | Ln 172, Col 46 | `java:S1604` | Make this anonymous inner class a lambda. | +| 18 | Ln 181, Col 18 | `java:S2142` | Either re-interrupt this method or rethrow the `InterruptedException` that can be caught here. | +| 19 | Ln 219, Col 46 | `java:S1604` | Make this anonymous inner class a lambda. | +| 20 | Ln 230, Col 18 | `java:S2147` | Combine this `catch` with the one at line 228, which has the same body. *(+1 location)* | +| 21 | Ln 230, Col 18 | `java:S2142` | Either re-interrupt this method or rethrow the `InterruptedException` that can be caught here. | +| 22 | Ln 247, Col 18 | `java:S2177` | Rename this method; there is a `private` method in the parent class with the same name. | + +> **Recurring themes in `DrawView`:** anonymous classes that should be lambdas (`S1604` ×3), swallowed `InterruptedException` (`S2142` ×2), and duplicated `catch` bodies (`S2147`). The `S2177` hidden-method and `S1141` nested-try findings are the higher-value maintainability ones. + +## 3. Notes for the portfolio + +- **Scope honestly.** Report smells in the classes the CR touches; mention the project-wide total (~592) only as context. +- **Connected Mode matters.** The quality profile of `domjab23` decides which rules fire and at what severity, so your exact list/severities are authoritative over the estimates above. +- **Tie smells back to the CR.** The `UndoAction`/`RedoAction` duplication (#12) is directly relevant to the worked CR (*add Undo All / Redo All*): adding `UndoAllAction`/`RedoAllAction` would tempt a third copy-paste, so the smell argues for extracting a shared superclass **before** implementing the feature. From 4a2ee014a516036edef252adc40e64865555f083 Mon Sep 17 00:00:00 2001 From: Klaus Fabo Date: Thu, 4 Jun 2026 15:13:48 +0200 Subject: [PATCH 15/18] refactoring undo/redo --- undo-redo/lab3-refactoring.md | 55 +++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 undo-redo/lab3-refactoring.md diff --git a/undo-redo/lab3-refactoring.md b/undo-redo/lab3-refactoring.md new file mode 100644 index 000000000..29d4d01c8 --- /dev/null +++ b/undo-redo/lab3-refactoring.md @@ -0,0 +1,55 @@ +# Lab 3 — Refactoring (remove the code smells) + +> **Project:** JHotDraw · **Feature / CR:** Undo/Redo +> **Input:** the smells found in [Lab 2](lab2-code-smells-sonarlint.md) · **Phase:** prefactoring (clean up *before* implementing the change) + +## Objective + +Apply one or more suitable **refactoring patterns** to remove bad code smells in the change-request classes, keeping behaviour unchanged (verified by the build + tests). + +## 1. Refactoring patterns applied + +| Pattern (Fowler) | Target | Smell(s) removed | +|------------------|--------|------------------| +| **Extract Superclass** (+ Pull Up Method / Pull Up Field) | `UndoAction` & `RedoAction` → new `AbstractUndoRedoAction` | #12 duplicated classes | +| **Rename Field** | `redoActionPropertyListener` → `actionPropertyListener` | #10 misleading name | +| **Replace Anonymous Class with Lambda / Method Reference** | the `PropertyChangeListener` → `this::realActionPropertyChanged` | #9 `java:S1604` | +| **Replace Wildcard Imports with Explicit Imports** | imports in all three action files | #8 `java:S2208` | +| Correct documentation during pull-up | copy-pasted "redo" wording in `UndoAction` Javadoc | #11 | + +> Scope: the three action classes in `org.jhotdraw.action.edit`. The smells in `UndoRedoManager` and `DrawView` are left for a later pass (or the actualization step), so this refactoring stays small and reviewable. + +## 2. What changed + +**New file:** `jhotdraw-actions/.../org/jhotdraw/action/edit/AbstractUndoRedoAction.java` +Holds the shared delegation logic that used to be duplicated: the property listener, `updateEnabledState`, `updateView`, `install/uninstallViewListeners`, `actionPerformed`, and `getRealAction()`. The only thing a subclass varies is the **action ID**, passed up through the constructor. + +**`UndoAction` / `RedoAction`** shrink from ~115 lines each to a thin subclass: + +```java +public class UndoAction extends AbstractUndoRedoAction { + private static final long serialVersionUID = 1L; + public static final String ID = "edit.undo"; + + public UndoAction(Application app, View view) { + super(app, view, ID); + } +} +``` + +(`RedoAction` is identical except `ID = "edit.redo"`.) + +### Behaviour preserved +- The public API is unchanged: both classes keep their `public static final String ID` and their `(Application, View)` constructor, so existing callers such as `DrawView` (`UndoAction.ID`, `RedoAction.ID`) are unaffected. +- The delegation logic is byte-for-byte the same, just pulled up; only the per-class `id` differs. + +## 3. Verification + +| Check | Result | +|-------|--------| +| `mvn -DskipTests install` | **BUILD SUCCESS** (compiles) | +| `mvn install` (full, with tests) | **BUILD SUCCESS**, all tests pass (`Tests run: … Failures: 0, Errors: 0`) | + +## 4. Why this is the right prefactoring for the CR + +The headline smell (#12) is **duplication between `UndoAction` and `RedoAction`**. The worked change request — *add Undo All / Redo All* — would otherwise tempt a **third** copy-paste (`UndoAllAction`/`RedoAllAction`). By extracting `AbstractUndoRedoAction` first, the new actions can extend the shared base and supply only their ID, so the feature is added without re-introducing the smell. This is prefactoring done for exactly the reason the methodology prescribes: clean the code on the change's path *before* changing it. From d0c4fa8f9b6c448015669bf066a107ca5e6b24eb Mon Sep 17 00:00:00 2001 From: Klaus Fabo Date: Thu, 4 Jun 2026 15:53:26 +0200 Subject: [PATCH 16/18] lab4 actualization, undo/redo --- undo-redo/lab4-actualization.md | 143 ++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 undo-redo/lab4-actualization.md diff --git a/undo-redo/lab4-actualization.md b/undo-redo/lab4-actualization.md new file mode 100644 index 000000000..d72272d5b --- /dev/null +++ b/undo-redo/lab4-actualization.md @@ -0,0 +1,143 @@ +# Lab 4 — Actualization (Clean Architecture & Clean Code) + +> **Project:** JHotDraw · **Feature / CR:** *Add an "Undo All / Redo All" command* +> **Phase:** Actualization — implement the new functionality, incorporate it into the old code, and propagate the change to every place that needs a secondary modification. +> **Builds on:** the impact set from [Lab 1](lab1-concept-location-and-impact-analysis.md) `{ UndoRedoManager, DrawView }` + the action classes, and the prefactoring in [Lab 3 — Refactoring](lab3-refactoring.md). + +## Introduction + +The **actualization** phase consists of implementing the new functionality, incorporating it into the old code, and **change propagation** that seeks out and updates all places in the old code that require secondary modification. The quality of that incorporation is governed by **Clean Architecture** (where code goes and which way dependencies point) and **Clean Code** principles (how each unit is written). + +## Objectives + +- Understand and explain **Clean Architecture** in the context of actualization. +- Understand and explain **Clean Code / SOLID principles** in the context of actualization. + +--- + +## Part 1 — SOLID principles in the case study + +Each principle is grounded in the actual undo/redo code (and in the prefactoring already applied). + +### S — Single Responsibility Principle (SRP) + +> *A class should have one reason to change.* + +The design **separates the gesture from the operation**: + +- `UndoAction` / `RedoAction` (`org.jhotdraw.action.edit`) — responsibility: **translate a menu gesture** into a call on the view's real action. They do **not** know how undo works. +- `UndoRedoManager` (`org.jhotdraw.undo`) — responsibility: **manage the edit stack** and perform `undo()`/`redo()`. + +**Actualization impact:** because responsibilities are split, *Undo All* is added in two well-defined places — a new manager method (stack policy) and a new menu action (gesture) — without one bleeding into the other. + +**Counter-example / smell:** `UndoRedoManager.setLocale()` (an instance method) writes the **static** `labels` field (Lab 2 smell #4, `java:S2696`) — mixing per-instance behaviour with global locale state. That is an SRP/encapsulation violation worth noting. + +### O — Open/Closed Principle (OCP) + +> *Open for extension, closed for modification.* + +This is the principle the **prefactoring (Lab 3 Refactoring) unlocked**. After **Extract Superclass**, `AbstractUndoRedoAction` holds the shared delegation logic, and a new command is added by **subclassing and supplying an ID** — no edit to existing code: + +```java +public class UndoAllAction extends AbstractUndoRedoAction { + public static final String ID = "edit.undoAll"; + public UndoAllAction(Application app, View view) { super(app, view, ID); } +} +``` + +`UndoRedoManager` is likewise open to new `UndoableEdit` types: it operates against the `UndoableEdit` abstraction, so new edit kinds need no manager change. + +### L — Liskov Substitution Principle (LSP) + +> *Subtypes must be usable wherever their base type is expected.* + +- `UndoRedoManager extends javax.swing.undo.UndoManager` and stays substitutable: it overrides `undo()`, `redo()`, `addEdit()` while preserving the contract (still throws `CannotUndoException`, still returns the right `boolean`). +- After prefactoring, `UndoAction`/`RedoAction` are substitutable wherever an `AbstractViewAction` / `Action` is expected — `DrawView` and the menu builder treat them uniformly. + +**Caveat:** `UndoRedoManager.redo()` declares `throws CannotUndoException` rather than `CannotRedoException` — a small contract wrinkle that slightly weakens strict LSP and is worth flagging in actualization. + +### I — Interface Segregation Principle (ISP) + +> *Clients should not depend on methods they do not use.* + +The collaboration is built on **small, focused interfaces**: + +- `UndoableEditListener` — a single method (`undoableEditHappened`); `UndoRedoManager` implements exactly what it needs to receive edits. +- `PropertyChangeListener` — one method; used to keep the menu action's name/enabled state in sync (now a lambda after prefactoring). +- `UndoableEdit` exposes a cohesive set (`undo`, `redo`, `canUndo`, `canRedo`, `die`) that every client genuinely uses. + +### D — Dependency Inversion Principle (DIP) + +> *Depend on abstractions, not concretions.* + +The menu controller **never references the concrete undo implementation**: + +```java +// UndoAction delegates through the abstraction javax.swing.Action, +// resolved by ID from the active view's ActionMap — not a hard reference +// to UndoRedoManager$UndoAction. +Action realAction = getActiveView().getActionMap().get(ID); +``` + +The concrete real action is **injected** by the view (`DrawView` registers `undo.getUndoAction()` under the ID). High-level UI depends on the `Action` abstraction; the low-level concrete action is supplied at runtime through the `ActionMap` (a string-keyed port). + +--- + +## Part 2 — Clean Architecture in the case study + +Robert C. Martin's Clean Architecture organises code into concentric layers with **the Dependency Rule: source-code dependencies point only inward**. Mapping the undo/redo feature onto those layers: + +``` + ┌──────────────────────────────────────────────┐ + │ Frameworks & Drivers (outer) │ + │ Swing: AbstractAction, UndoManager, JMenu; │ + │ DrawView, application model / menu wiring │ + │ ┌────────────────────────────────────────┐ │ + │ │ Interface Adapters / Controllers │ │ + │ │ UndoAction / RedoAction │ │ + │ │ (AbstractUndoRedoAction) │ │ + │ │ ┌────────────────────────────────┐ │ │ + │ │ │ Use Cases / Application │ │ │ + │ │ │ UndoRedoManager.undo()/redo() │ │ │ + │ │ │ ┌────────────────────────┐ │ │ │ + │ │ │ │ Entities (core) │ │ │ │ + │ │ │ │ edit stack, │ │ │ │ + │ │ │ │ UndoableEdit objects │ │ │ │ + │ │ │ └────────────────────────┘ │ │ │ + │ │ └────────────────────────────────┘ │ │ + │ └────────────────────────────────────────┘ │ + └──────────────────────────────────────────────┘ + dependencies point inward ↑ +``` + +| Clean-Architecture layer | Undo/redo code | Role | +|--------------------------|----------------|------| +| **Entities** (enterprise policy) | `UndoableEdit` / `AbstractUndoableEdit` / `CompositeEdit` and the edit stack | Encapsulate a change and how to reverse/reapply it — the most stable policy. | +| **Use cases** (application policy) | `UndoRedoManager.undo()/redo()/addEdit()` | Orchestrate the edit stack to carry out the undo/redo operation. | +| **Interface adapters** | `UndoAction` / `RedoAction` → `AbstractUndoRedoAction` | Translate a GUI gesture into a use-case call; adapt Swing's `Action` world to the manager. | +| **Frameworks & drivers** | Swing (`AbstractAction`, `UndoManager`, menus), `DrawView`, application model | The volatile outer ring — UI toolkit and view wiring. | + +**The Dependency Rule holds here.** The core `UndoRedoManager` knows nothing about menus or `DrawView`; the outward menu action depends *inward* on the `Action` abstraction. The crossing of the boundary uses a **port** — the `ActionMap` keyed by a string ID — so the UI never statically depends on the undo implementation (this is the architectural face of DIP above). Each document gets its **own per-view manager**, so a use-case instance is scoped to its document rather than being a global singleton. + +### How Clean Architecture guides the actualization + +Implementing *Undo All / Redo All* slots cleanly into the layers — and the **change propagates outward, never inward**: + +1. **Entities/Use cases:** add `undoAll()` / `redoAll()` to `UndoRedoManager` (loop `undo()` while `canUndo()`), bracketed by the existing `undoOrRedoInProgress` guard. *(core — CHANGED in impact analysis)* +2. **Interface adapters:** add `UndoAllAction` / `RedoAllAction` extending `AbstractUndoRedoAction` with new IDs (OCP — no existing adapter modified). +3. **Frameworks & drivers:** register the real actions in `DrawView`'s `ActionMap` and add the menu items in the application model. *(view — CHANGED in impact analysis)* + +No inner layer is forced to know about an outer one, which is exactly why the impact set stayed small. + +### Clean Code principles applied during actualization + +- **DRY** — the **Extract Superclass** prefactoring removed the `UndoAction`/`RedoAction` duplication so the feature does not copy-paste a third time. +- **Meaningful names** — **Rename Field** fixed the misleading `redoActionPropertyListener` inside `UndoAction`. +- **Small, single-purpose units** — `AbstractUndoRedoAction` does one thing (delegate); subclasses only declare identity. +- **Prefer abstractions over console I/O** — Lab 2 flagged `System.out`/`printStackTrace` (`java:S106`/`S1148`); replacing them with a logger is the remaining Clean-Code cleanup. + +--- + +## Summary + +Actualization is not just "write the feature" — it is incorporating it so that **dependencies still point inward** (Clean Architecture) and each unit stays **single-purpose, DRY, and well-named** (Clean Code / SOLID). For *Undo All / Redo All*, the prefactoring (OCP via Extract Superclass) means the new commands are added at the adapter layer plus a single core method, with the change propagating outward through `DrawView` only — matching the `{ UndoRedoManager, DrawView }` impact set predicted in Lab 1. From c4795f982487fdd5caa197ff36089478abf2fa06 Mon Sep 17 00:00:00 2001 From: Klaus Fabo Date: Fri, 5 Jun 2026 12:00:51 +0200 Subject: [PATCH 17/18] Unit testing undo/redo --- jhotdraw-utils/pom.xml | 12 ++ .../jhotdraw/undo/UndoRedoManagerTest.java | 184 ++++++++++++++++++ .../org/jhotdraw/undo/Labels.properties | 5 + undo-redo/lab5-testing.md | 73 +++++++ 4 files changed, 274 insertions(+) create mode 100644 jhotdraw-utils/src/test/java/org/jhotdraw/undo/UndoRedoManagerTest.java create mode 100644 jhotdraw-utils/src/test/resources/org/jhotdraw/undo/Labels.properties create mode 100644 undo-redo/lab5-testing.md diff --git a/jhotdraw-utils/pom.xml b/jhotdraw-utils/pom.xml index b1b2faf01..60db608d1 100644 --- a/jhotdraw-utils/pom.xml +++ b/jhotdraw-utils/pom.xml @@ -15,5 +15,17 @@ 6.8.21 test + + junit + junit + 4.13.2 + test + + + org.mockito + mockito-core + 5.11.0 + test + \ No newline at end of file diff --git a/jhotdraw-utils/src/test/java/org/jhotdraw/undo/UndoRedoManagerTest.java b/jhotdraw-utils/src/test/java/org/jhotdraw/undo/UndoRedoManagerTest.java new file mode 100644 index 000000000..f86df3add --- /dev/null +++ b/jhotdraw-utils/src/test/java/org/jhotdraw/undo/UndoRedoManagerTest.java @@ -0,0 +1,184 @@ +/* + * @(#)UndoRedoManagerTest.java + * + * Unit tests for the core domain logic of the Undo/Redo feature. + */ +package org.jhotdraw.undo; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import javax.swing.undo.AbstractUndoableEdit; +import javax.swing.undo.CannotRedoException; +import javax.swing.undo.CannotUndoException; +import javax.swing.undo.UndoableEdit; +import org.junit.Before; +import org.junit.Test; + +/** + * JUnit 4 unit tests for {@link UndoRedoManager} — the edit-stack engine that is + * the core domain logic of the Undo/Redo feature. + *

+ * Tests cover the best-case flow (add → undo → redo), the boundary cases + * (empty stack, nothing to redo, re-entrant edits during undo), and use a + * Mockito stub to isolate the manager from a concrete {@link UndoableEdit}. + */ +public class UndoRedoManagerTest { + + private UndoRedoManager manager; + + /** + * A minimal test stub: a significant edit that records how many times it was + * undone/redone. Used instead of mocking when we want real stack behaviour. + */ + private static final class CountingEdit extends AbstractUndoableEdit { + private static final long serialVersionUID = 1L; + int undoCount = 0; + int redoCount = 0; + + @Override + public void undo() { + super.undo(); + undoCount++; + } + + @Override + public void redo() { + super.redo(); + redoCount++; + } + } + + @Before + public void setUp() { + manager = new UndoRedoManager(); + } + + // ---------------------------------------------------------------- best case + + @Test + public void newManagerHasNothingToUndoOrRedo() { + assertFalse("a fresh manager cannot undo", manager.canUndo()); + assertFalse("a fresh manager cannot redo", manager.canRedo()); + assertFalse("a fresh manager has no significant edits", manager.hasSignificantEdits()); + } + + @Test + public void addingSignificantEditEnablesUndo() { + manager.addEdit(new CountingEdit()); + + assertTrue("after adding an edit, undo is possible", manager.canUndo()); + assertTrue("a significant edit was recorded", manager.hasSignificantEdits()); + assertFalse("nothing has been undone yet, so redo is not possible", manager.canRedo()); + } + + @Test + public void undoReversesLastEdit() { + CountingEdit edit = new CountingEdit(); + manager.addEdit(edit); + + manager.undo(); + + assertEquals("undo() is delegated to the edit exactly once", 1, edit.undoCount); + assertFalse("nothing left to undo", manager.canUndo()); + assertTrue("the undone edit can now be redone", manager.canRedo()); + } + + @Test + public void redoReappliesUndoneEdit() { + CountingEdit edit = new CountingEdit(); + manager.addEdit(edit); + manager.undo(); + + manager.redo(); + + assertEquals("redo() is delegated to the edit exactly once", 1, edit.redoCount); + assertTrue("the redone edit can be undone again", manager.canUndo()); + assertFalse("nothing left to redo", manager.canRedo()); + } + + @Test + public void discardAllEditsClearsHistory() { + manager.addEdit(new CountingEdit()); + + manager.discardAllEdits(); + + assertFalse("history is empty after discardAllEdits", manager.canUndo()); + assertFalse("significant-edits flag is reset", manager.hasSignificantEdits()); + } + + // ----------------------------------------------------------- boundary cases + + @Test(expected = CannotUndoException.class) + public void undoOnEmptyStackThrows() { + manager.undo(); // nothing was added -> must throw + } + + @Test(expected = CannotRedoException.class) + public void redoWithNothingToRedoThrows() { + manager.redo(); // nothing was undone -> must throw + } + + @Test + public void editsArrivingWhileUndoInProgressAreIgnored() { + // Boundary: the manager must ignore edits fired *during* an undo + // (undoOrRedoInProgress guard), otherwise the stack would corrupt. + final CountingEdit latecomer = new CountingEdit(); + UndoableEdit reentrantEdit = new AbstractUndoableEdit() { + private static final long serialVersionUID = 1L; + + @Override + public void undo() { + super.undo(); + manager.addEdit(latecomer); // fired in the middle of an undo + } + }; + manager.addEdit(reentrantEdit); + + manager.undo(); + + // If the latecomer had been accepted, canUndo() would be true. + assertFalse("re-entrant edit must be ignored, leaving nothing to undo", manager.canUndo()); + assertEquals("the ignored edit is never undone", 0, latecomer.undoCount); + } + + // -------------------------------------------------- mockito (isolation) test + + @Test + public void undoDelegatesReversalToTheEdit() { + // Stub a UndoableEdit so the manager is tested in isolation from any + // concrete edit implementation (mockito.org). We only program the + // collaborator's contract, then verify the manager calls undo() on it. + UndoableEdit edit = mock(UndoableEdit.class); + when(edit.isSignificant()).thenReturn(true); + when(edit.canUndo()).thenReturn(true); + + manager.addEdit(edit); + manager.undo(); + + verify(edit).undo(); // the manager delegates the actual reversal to the edit + } + + // ------------------------------------------------- java assertion (invariant) + + @Test + public void undoRedoCountsStayBalanced() { + CountingEdit edit = new CountingEdit(); + manager.addEdit(edit); + manager.undo(); + manager.redo(); + + // JAVA assertion: an invariant that must NEVER be violated. Surefire runs + // tests with assertions enabled (-ea), so this is enforced; if the + // manager ever redid more than it undid, the program would halt here. + assert edit.redoCount <= edit.undoCount + 1 + : "invariant violated: redo count outran undo count"; + + assertEquals(1, edit.undoCount); + assertEquals(1, edit.redoCount); + } +} diff --git a/jhotdraw-utils/src/test/resources/org/jhotdraw/undo/Labels.properties b/jhotdraw-utils/src/test/resources/org/jhotdraw/undo/Labels.properties new file mode 100644 index 000000000..11b3ab5e1 --- /dev/null +++ b/jhotdraw-utils/src/test/resources/org/jhotdraw/undo/Labels.properties @@ -0,0 +1,5 @@ +# Minimal test-only resource bundle so UndoRedoManager can construct in +# isolated jhotdraw-utils unit tests (the production bundle lives in +# jhotdraw-core, which is not on this module's test classpath). +edit.undo.text=Undo +edit.redo.text=Redo diff --git a/undo-redo/lab5-testing.md b/undo-redo/lab5-testing.md new file mode 100644 index 000000000..61b60703f --- /dev/null +++ b/undo-redo/lab5-testing.md @@ -0,0 +1,73 @@ +# Lab 5 — Testing (Unit Tests for the Undo/Redo Feature) + +> **Project:** JHotDraw · **Feature / CR:** Undo/Redo +> **Unit under test:** `UndoRedoManager` (`org.jhotdraw.undo`) — the edit-stack engine that is the core domain logic of the feature. + +## Introduction + +Unit testing verifies individual units of source code in isolation to determine they behave as intended. In object-oriented code a unit is usually a class or a method. By testing the smallest units first, then their compound behaviours, we build up confidence in the whole feature. + +## Objectives + +- Understand the importance of testing. +- Implement unit tests for the feature's most important domain logic. + +## What was tested and why + +The feature's domain logic lives in **`UndoRedoManager`** (located in Lab 1, marked CHANGED in the impact analysis). The menu actions (`UndoAction`/`RedoAction`) are thin UI controllers that only delegate, so the behaviour worth verifying — the edit stack, the undo/redo operations, and the in-progress guard — is all in the manager. + +## Classwork — how each requirement was met + +| Lab step | Done | +|----------|------| +| 1. Add JUnit 4 dependency | Added `junit:junit:4.13.2` (test scope) to `jhotdraw-utils/pom.xml`. | +| 2. JUnit 4 tests for domain logic | `UndoRedoManagerTest` in `jhotdraw-utils/src/test/java/org/jhotdraw/undo/`. | +| 3. Best-case tests | add → undo → redo flow (see below). | +| 4. Boundary-case tests | empty-stack undo, nothing-to-redo, re-entrant edit during undo. | +| 4a. Mocks/stubs for dependencies | `mockito-core:5.11.0` used to stub a `UndoableEdit` so the manager is tested in isolation. | +| 5. Java assertions for invariants | a `assert` statement checks an undo/redo balance invariant (Surefire runs tests with `-ea`). | + +### Test inventory + +**Best case** +- `newManagerHasNothingToUndoOrRedo` — a fresh manager cannot undo/redo. +- `addingSignificantEditEnablesUndo` — adding an edit enables undo and sets `hasSignificantEdits`. +- `undoReversesLastEdit` — `undo()` delegates to the edit once; redo becomes available. +- `redoReappliesUndoneEdit` — `redo()` delegates once; undo becomes available again. +- `discardAllEditsClearsHistory` — history and the significant-edits flag reset. + +**Boundary** +- `undoOnEmptyStackThrows` — `undo()` on an empty stack throws `CannotUndoException` (`@Test(expected = …)`). +- `redoWithNothingToRedoThrows` — `redo()` with nothing undone throws `CannotRedoException`. +- `editsArrivingWhileUndoInProgressAreIgnored` — an edit fired *during* an undo is ignored (the `undoOrRedoInProgress` guard), leaving nothing to undo. + +**Isolation (Mockito)** +- `undoDelegatesReversalToTheEdit` — stubs `UndoableEdit` (`isSignificant()`/`canUndo()` → `true`) and `verify(edit).undo()` confirms the manager delegates the actual reversal to the edit, without depending on a concrete edit class. + +**Invariant (Java assertion)** +- `undoRedoCountsStayBalanced` — uses `assert edit.redoCount <= edit.undoCount + 1` to enforce an invariant that should never break, in addition to the JUnit assertions on the exact counts. + +### Note on assertions (lab step 5) + +Two kinds of assertion are used deliberately, per the brief: +- **JUnit assertions** (`assertEquals`, `assertTrue`, …) verify the *expected outcome* of a test and fail the test when violated. +- A **Java `assert`** checks an *invariant* — "something that should never happen." If violated it halts execution (an `AssertionError`), unlike an exception which is part of normal control flow. Surefire enables assertions by default, so the `assert` is active during the test run. + +### Isolating dependencies (lab step 4a) + +A unit test should exercise a single code-path through a single method; when execution leaves that method you have a dependency that should be stubbed. Two techniques are shown: +- **Hand-written stub** — `CountingEdit extends AbstractUndoableEdit` records undo/redo counts (used where real stack semantics are needed). +- **Mockito mock** — `mock(UndoableEdit.class)` with programmed behaviour (used to verify pure delegation in isolation). + +## Verification — results + +| Command | Result | +|---------|--------| +| `mvn -pl jhotdraw-utils test` | **BUILD SUCCESS** — `Tests run: 14, Failures: 0, Errors: 0, Skipped: 0` (10 new JUnit + 4 existing TestNG). | +| `mvn install` (full reactor, with tests) | **BUILD SUCCESS** — all module tests pass. | + +> **Coexistence note:** the module already used **TestNG** (`BezierPathNGTest`). Adding JUnit 4 did not displace it — Surefire ran both, so the existing tests keep passing alongside the new ones. + +## How the feature was verified (summary for the portfolio) + +The Undo/Redo feature's core behaviour is verified by automated JUnit 4 tests on `UndoRedoManager`: the best-case add/undo/redo cycle, the error boundaries (empty stack, nothing to redo), and the re-entrancy guard. Dependencies on concrete edits are removed with a Mockito stub so each test isolates one code-path, and a Java assertion guards a core invariant. All 14 module tests pass, and the full project build is green — giving objective evidence the feature behaves as designed. From 35722653b4ec1cf95ffb5a2385cb6899c715d2a5 Mon Sep 17 00:00:00 2001 From: Klaus Fabo Date: Fri, 5 Jun 2026 12:22:45 +0200 Subject: [PATCH 18/18] Functional testing undo/redo --- jhotdraw-utils/pom.xml | 12 ++ .../jhotdraw/undo/bdd/UndoRedoBddTest.java | 179 ++++++++++++++++++ undo-redo/lab6-bdd-testing.md | 93 +++++++++ 3 files changed, 284 insertions(+) create mode 100644 jhotdraw-utils/src/test/java/org/jhotdraw/undo/bdd/UndoRedoBddTest.java create mode 100644 undo-redo/lab6-bdd-testing.md diff --git a/jhotdraw-utils/pom.xml b/jhotdraw-utils/pom.xml index 60db608d1..b219cf462 100644 --- a/jhotdraw-utils/pom.xml +++ b/jhotdraw-utils/pom.xml @@ -27,5 +27,17 @@ 5.11.0 test + + com.tngtech.jgiven + jgiven-junit + 1.3.1 + test + + + org.assertj + assertj-core + 3.25.3 + test + \ No newline at end of file diff --git a/jhotdraw-utils/src/test/java/org/jhotdraw/undo/bdd/UndoRedoBddTest.java b/jhotdraw-utils/src/test/java/org/jhotdraw/undo/bdd/UndoRedoBddTest.java new file mode 100644 index 000000000..426de8df7 --- /dev/null +++ b/jhotdraw-utils/src/test/java/org/jhotdraw/undo/bdd/UndoRedoBddTest.java @@ -0,0 +1,179 @@ +/* + * @(#)UndoRedoBddTest.java + * + * Behaviour-Driven tests for the Undo/Redo feature, written as JGiven + * Given-When-Then scenarios that mirror the feature's user stories. + */ +package org.jhotdraw.undo.bdd; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.tngtech.jgiven.Stage; +import com.tngtech.jgiven.annotation.ScenarioState; +import com.tngtech.jgiven.junit.ScenarioTest; +import javax.swing.undo.AbstractUndoableEdit; +import javax.swing.undo.CannotUndoException; +import org.jhotdraw.undo.UndoRedoManager; +import org.junit.Test; + +/** + * BDD scenarios for the Undo/Redo feature, automated with JGiven and AssertJ. + * + *

User stories mapped to scenarios:

+ *
    + *
  • US-1 — As a drawing user, I want to undo my last change so that I + * can correct mistakes.
    • + *
  • US-2 — As a drawing user, I want to redo a change I undid so that + * I can reapply it.
    • + *
  • US-3 — As a drawing user, I want Undo to be safe when there is + * nothing to undo so that the application does not crash.
    • + *
+ */ +public class UndoRedoBddTest extends ScenarioTest< + UndoRedoBddTest.GivenAnUndoManager, + UndoRedoBddTest.WhenTheUser, + UndoRedoBddTest.ThenTheOutcome> { + + // US-1: Given an edit has been recorded, When the user undoes, Then it is reversed. + @Test + public void user_can_undo_the_last_change() { + given().an_undo_manager() + .and().an_edit_has_been_recorded(); + when().the_user_triggers_undo(); + then().the_last_edit_is_reversed() + .and().redo_becomes_available(); + } + + // US-2: Given an edit has been undone, When the user redoes, Then it is reapplied. + @Test + public void user_can_redo_an_undone_change() { + given().an_undo_manager() + .and().an_edit_has_been_recorded() + .and().the_edit_has_been_undone(); + when().the_user_triggers_redo(); + then().the_last_edit_is_reapplied() + .and().undo_becomes_available_again(); + } + + // US-3 (boundary): Given nothing recorded, When the user undoes, Then it is rejected. + @Test + public void undo_is_rejected_when_there_is_nothing_to_undo() { + given().an_undo_manager(); + when().the_user_triggers_undo_expecting_failure(); + then().the_undo_is_rejected_safely(); + } + + /** Test double: a significant edit that records how often it is undone/redone. */ + public static final class RecordingEdit extends AbstractUndoableEdit { + private static final long serialVersionUID = 1L; + int undoCount = 0; + int redoCount = 0; + + @Override + public void undo() { + super.undo(); + undoCount++; + } + + @Override + public void redo() { + super.redo(); + redoCount++; + } + } + + // ---------------------------------------------------------------- GIVEN stage + + public static class GivenAnUndoManager extends Stage { + + @ScenarioState + UndoRedoManager manager; + @ScenarioState + RecordingEdit lastEdit; + + public GivenAnUndoManager an_undo_manager() { + manager = new UndoRedoManager(); + return self(); + } + + public GivenAnUndoManager an_edit_has_been_recorded() { + lastEdit = new RecordingEdit(); + manager.addEdit(lastEdit); + return self(); + } + + public GivenAnUndoManager the_edit_has_been_undone() { + manager.undo(); + return self(); + } + } + + // ----------------------------------------------------------------- WHEN stage + + public static class WhenTheUser extends Stage { + + @ScenarioState + UndoRedoManager manager; + @ScenarioState + CannotUndoException caughtUndoError; + + public WhenTheUser the_user_triggers_undo() { + manager.undo(); + return self(); + } + + public WhenTheUser the_user_triggers_redo() { + manager.redo(); + return self(); + } + + public WhenTheUser the_user_triggers_undo_expecting_failure() { + try { + manager.undo(); + } catch (CannotUndoException e) { + caughtUndoError = e; + } + return self(); + } + } + + // ----------------------------------------------------------------- THEN stage + + public static class ThenTheOutcome extends Stage { + + @ScenarioState + UndoRedoManager manager; + @ScenarioState + RecordingEdit lastEdit; + @ScenarioState + CannotUndoException caughtUndoError; + + public ThenTheOutcome the_last_edit_is_reversed() { + assertThat(lastEdit.undoCount).as("edit was undone once").isEqualTo(1); + return self(); + } + + public ThenTheOutcome redo_becomes_available() { + assertThat(manager.canRedo()).as("redo is now possible").isTrue(); + return self(); + } + + public ThenTheOutcome the_last_edit_is_reapplied() { + assertThat(lastEdit.redoCount).as("edit was redone once").isEqualTo(1); + return self(); + } + + public ThenTheOutcome undo_becomes_available_again() { + assertThat(manager.canUndo()).as("undo is possible again").isTrue(); + return self(); + } + + public ThenTheOutcome the_undo_is_rejected_safely() { + assertThat(caughtUndoError) + .as("undo on an empty stack is rejected with CannotUndoException") + .isInstanceOf(CannotUndoException.class); + assertThat(manager.canUndo()).as("still nothing to undo").isFalse(); + return self(); + } + } +} diff --git a/undo-redo/lab6-bdd-testing.md b/undo-redo/lab6-bdd-testing.md new file mode 100644 index 000000000..e9b3e6e04 --- /dev/null +++ b/undo-redo/lab6-bdd-testing.md @@ -0,0 +1,93 @@ +# Lab 6 — Behaviour-Driven Testing (User Stories → JGiven Scenarios) + +> **Project:** JHotDraw · **Feature / CR:** Undo/Redo +> **Tools:** [JGiven](https://jgiven.org) (Given-When-Then automation) + [AssertJ](https://assertj.github.io/doc/) (fluent assertions) +> **Unit under test:** `UndoRedoManager` — the core domain logic of the feature. + +## Introduction — User Stories and BDD + +A **user story** is a short description of a capability from the user's perspective: +*"As a [user type], I want [some goal] so that [some reason]."* +In **Behaviour-Driven Development** each story is turned into one or more **Given-When-Then** scenarios that become executable tests. + +## Part 1 — User stories mapped to BDD scenarios + +| # | User story | BDD scenario (Given-When-Then) | +|---|------------|--------------------------------| +| **US-1** | As a drawing user, I want to **undo** my last change so that I can correct mistakes. | **Given** an undo manager with one recorded edit · **When** the user triggers Undo · **Then** the edit is reversed **and** Redo becomes available. | +| **US-2** | As a drawing user, I want to **redo** a change I undid so that I can reapply it. | **Given** an edit has been recorded and undone · **When** the user triggers Redo · **Then** the edit is reapplied **and** Undo becomes available again. | +| **US-3** | As a drawing user, I want Undo to be **safe when there is nothing to undo** so that the app does not crash. | **Given** a fresh undo manager · **When** the user triggers Undo · **Then** the undo is rejected safely (`CannotUndoException`) and nothing changes. | + +## Part 2 — Automating the scenarios with JGiven + +JGiven structures each scenario into three **stages** whose step methods read like the Given-When-Then text. State is shared between stages with `@ScenarioState`. + +**Test:** `jhotdraw-utils/src/test/java/org/jhotdraw/undo/bdd/UndoRedoBddTest.java` + +```java +public class UndoRedoBddTest extends ScenarioTest { + + @Test + public void user_can_undo_the_last_change() { + given().an_undo_manager() + .and().an_edit_has_been_recorded(); + when().the_user_triggers_undo(); + then().the_last_edit_is_reversed() + .and().redo_becomes_available(); + } + // … US-2, US-3 … +} +``` + +The stages: + +| Stage | Steps | +|-------|-------| +| `GivenAnUndoManager` | `an_undo_manager()`, `an_edit_has_been_recorded()`, `the_edit_has_been_undone()` | +| `WhenTheUser` | `the_user_triggers_undo()`, `the_user_triggers_redo()`, `the_user_triggers_undo_expecting_failure()` | +| `ThenTheOutcome` | `the_last_edit_is_reversed()`, `redo_becomes_available()`, `the_last_edit_is_reapplied()`, `undo_becomes_available_again()`, `the_undo_is_rejected_safely()` | + +## Part 3 — Domain assertions with AssertJ + +The `Then` steps use **AssertJ** fluent, self-describing assertions: + +```java +assertThat(lastEdit.undoCount).as("edit was undone once").isEqualTo(1); +assertThat(manager.canRedo()).as("redo is now possible").isTrue(); +assertThat(caughtUndoError) + .as("undo on an empty stack is rejected with CannotUndoException") + .isInstanceOf(CannotUndoException.class); +``` + +### Why AssertJ-core (not AssertJ-Swing) here + +The lab notes **AssertJ-Swing** for driving Swing UIs. That tool automates the GUI via an AWT `Robot` and **needs a display**, so it suits *end-to-end* scenarios that click `Edit → Undo` in the running app (`DrawView`/`Main`). The behaviour worth verifying for this feature lives in the **headless domain object** `UndoRedoManager`, so these scenarios test it directly with **AssertJ-core** — faster, deterministic, and runnable in CI without a display. AssertJ-Swing would be the right tool for a follow-up GUI scenario layer. + +## Dependencies added (`jhotdraw-utils/pom.xml`) + +```xml + + com.tngtech.jgiven + jgiven-junit + 1.3.1 + test + + + org.assertj + assertj-core + 3.25.3 + test + +``` + +## Verification — results + +| Command | Result | +|---------|--------| +| `mvn -pl jhotdraw-utils test` | **BUILD SUCCESS** — `Tests run: 17, Failures: 0, Errors: 0` (3 JGiven scenarios + 10 JUnit + 4 TestNG). | + +JGiven also writes a structured report to `target/jgiven-reports/` (JSON; an HTML report can be generated from it), giving living documentation of the scenarios straight from the test run. + +## Summary + +The Undo/Redo feature's user stories (US-1 undo, US-2 redo, US-3 safe-empty-undo) are expressed as executable **Given-When-Then** scenarios with **JGiven**, asserted with **AssertJ**, and run green alongside the Lab 5 unit tests. Together, Lab 5 (unit) and Lab 6 (behaviour) verify the feature both at the method level and at the user-story level.