Skip to content

Commit 4e51535

Browse files
Address code review feedback: use IsRemoved flag and add comments
Co-authored-by: RuntimeRascal <2422222+RuntimeRascal@users.noreply.github.com>
1 parent d25e1ea commit 4e51535

File tree

2 files changed

+10
-5
lines changed

2 files changed

+10
-5
lines changed

Src/GhostDraw/Services/DrawingHistory.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public void RecordAction(UIElement element)
7676
_elementIdToEntry.Remove(entry.Id);
7777

7878
// Check if element is still valid (not already removed)
79-
if (entry.Element != null)
79+
if (!entry.IsRemoved)
8080
{
8181
_logger.LogInformation("Undo: Removing element ID={Id}, Type={Type}, RemainingActions={Count}",
8282
entry.Id, entry.Element.GetType().Name, _undoStack.Count);
@@ -85,7 +85,7 @@ public void RecordAction(UIElement element)
8585
else
8686
{
8787
// Element was already removed (e.g., by eraser), skip to next
88-
_logger.LogDebug("Undo: Skipping null element ID={Id} (already removed)", entry.Id);
88+
_logger.LogDebug("Undo: Skipping removed element ID={Id}", entry.Id);
8989
}
9090
}
9191

@@ -112,8 +112,8 @@ public void RemoveFromHistory(UIElement element)
112112
{
113113
if (_elementIdToEntry.TryGetValue(id, out var entry))
114114
{
115-
// Mark the entry's element as null so it will be skipped during undo
116-
entry.Element = null;
115+
// Mark the entry as removed so it will be skipped during undo
116+
entry.IsRemoved = true;
117117
_elementIdToEntry.Remove(id);
118118

119119
_logger.LogDebug("Element removed from history: ID={Id}, Type={Type}",
@@ -168,12 +168,14 @@ public void Clear()
168168
private class HistoryEntry
169169
{
170170
public Guid Id { get; }
171-
public UIElement? Element { get; set; }
171+
public UIElement Element { get; }
172+
public bool IsRemoved { get; set; }
172173

173174
public HistoryEntry(Guid id, UIElement element)
174175
{
175176
Id = id;
176177
Element = element;
178+
IsRemoved = false;
177179
}
178180
}
179181
}

Src/GhostDraw/Tools/EraserTool.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ public class EraserTool(ILogger<EraserTool> logger) : IDrawingTool
3030
private double _currentThickness = 3.0;
3131
private readonly HashSet<UIElement> _erasedElements = new();
3232

33+
// ActionCompleted event required by IDrawingTool but not used by eraser.
34+
// Eraser uses ElementErased event instead since it removes existing elements
35+
// rather than creating new ones.
3336
public event EventHandler<DrawingActionCompletedEventArgs>? ActionCompleted;
3437

3538
/// <summary>

0 commit comments

Comments
 (0)