CompositeCommand enumerates its commands argument three times: single-pass sequences throw "must contain at least one command" and Select factories run 3×

[matt-edmondson]

What's wrong

CompositeCommand's constructor (UndoRedo/CompositeCommand.cs ~L26-34) enumerates commands three separate times:

1. GetAffectedItems(commands) in the base(...) call → [.. commands]
2. GetTotalSize(commands) in the base(...) call → [.. commands]
3. _commands = [.. commands] in the body

Failure scenarios (reproduced with temporary MSTest tests)

Single-pass source → spurious exception

IEnumerable<ICommand> Drain(Queue<ICommand> q) { while (q.Count > 0) yield return q.Dequeue(); }
new CompositeCommand("batch", Drain(pending)); // pending holds 2 commands

The first enumeration drains the queue, _commands ends up empty, and the constructor throws ArgumentException: Composite command must contain at least one command (Parameter 'commands') even though two commands were supplied. Same with BlockingCollection.GetConsumingEnumerable() or anything stream-backed.

Projected source → three different sets of commands

new CompositeCommand("batch", Enumerable.Range(0, 2).Select(i => new DelegateCommand(...)));

The factory runs 6 times instead of 2. Metadata.AffectedItems and Metadata.Size are computed from two throwaway sets of commands, while the composite executes/undoes a third set. Any side effects in the factory (ID allocation, capturing current state) happen three times, and metadata can disagree with what's actually executed.

Suggested fix

Materialize once and derive everything from that list — e.g. a private constructor taking List<ICommand> that the public one chains to via commands.ToList(), or compute affected items / size from _commands. While there, reject null elements with an ArgumentException (today a null element surfaces as a NullReferenceException inside GetAffectedItems).

Acceptance criteria

• Constructing from a single-pass iterator of N commands yields a composite with N commands and no exception.
• A counting Select factory is invoked exactly N times.
• A null element throws ArgumentException.

[matt-edmondson]

Triage

• Category: Bug
• Priority: Medium. Building a composite from a single-pass sequence throws a misleading "must contain at least one command". A Select factory runs three times, so the metadata can disagree with the commands that actually execute. Most callers pass a list or array and won't notice, but the failure is confusing when it does happen.
• Area / suggested assignee: CompositeCommand constructor, GetAffectedItems and GetTotalSize (UndoRedo/CompositeCommand.cs). Owner: @matt-edmondson
• Duplicates / in progress: not a duplicate, and no open PR covers it. It is the same multiple-enumeration pattern as Join/ToStringEnumerable with NullItemHandling.Throw enumerate the source twice: one-shot sequences join to "" and late nulls slip through Extensions#138.
• Next step: materialize commands once, for example with a private constructor that takes List<ICommand>, and compute affected items and size from that list. Reject null elements with ArgumentException. Add tests for a single-pass iterator, a counting factory that runs exactly N times, and a null element.

---

Generated by Claude Code