From 4cdb048dfc2ceed59b8f5c54f5db39c1823fa2a3 Mon Sep 17 00:00:00 2001 From: Willem van den Ende Date: Sat, 13 Jun 2026 15:44:46 +0100 Subject: [PATCH] refactor(story2): immutable dealDamage, reference guard, negative damage validation, spec precision - dealDamage returns new Character instead of mutating in-place - SelfDamageForbidden uses reference equality (this === target) - Negative damage throws at the boundary - Removed duplicate Health.maxHealthForLevel (Level.ts is source of truth) - Allium spec uses max(0, ...) for health floor precision - New property: NegativeDamageForbidden (11 total properties) --- specs/damage-and-health.allium | 4 +-- src/Character.ts | 27 ++++++++------- src/Health.ts | 4 --- src/damage-and-health.spec.ts | 63 ++++++++++++++++++++++++---------- 4 files changed, 62 insertions(+), 36 deletions(-) diff --git a/specs/damage-and-health.allium b/specs/damage-and-health.allium index f15b6f2..7789c3a 100644 --- a/specs/damage-and-health.allium +++ b/specs/damage-and-health.allium @@ -10,9 +10,9 @@ rule DamageReducesHealth { when: Character.dealDamage(attacker, target, damage) requires: attacker.name != target.name requires: target.status = alive - ensures: target.health.value = target.health.value - damage + ensures: target.health.value = max(0, target.health.value - damage) ensures: - if target.health.value - damage <= 0: + if max(0, target.health.value - damage) = 0: target.status = dead else: target.status = alive diff --git a/src/Character.ts b/src/Character.ts index 6fd76dd..77b1000 100644 --- a/src/Character.ts +++ b/src/Character.ts @@ -24,9 +24,11 @@ export interface CharacterCtorWithHealth { export class Character { #state: CharacterState; + readonly #name: string; private constructor(state: CharacterState) { this.#state = state; + this.#name = state.name; } /** Create a new character with default health (1000) and alive status. */ @@ -42,7 +44,7 @@ export class Character { } get name(): string { - return this.#state.name; + return this.#name; } get health(): Health { @@ -61,21 +63,22 @@ export class Character { return this.#state.factions; } - /** Deal damage to another character. Mutates target in place. */ - dealDamage(target: Character, damage: number): void { - // Self-damage is forbidden - if (this.name === target.name) return; + /** + * Deal damage to another character. Returns a new Character with updated state. + * Does not mutate the attacker or the original target reference. + */ + dealDamage(target: Character, damage: number): Character { + // Self-damage is forbidden — use reference equality, not name + if (this === target) return target; // Dead characters cannot take damage - if (target.status.kind === 'dead') return; + if (target.status.kind === 'dead') return target; + // Negative damage is invalid + if (damage < 0) throw new Error(`Damage must be non-negative, got ${damage}`); // Reduce health const newHealth = target.health.sub(damage); const newStatus = newHealth.value === 0 ? StatusDead : StatusAlive; - target.#state = new CharacterState( - target.name, - newHealth, - newStatus, - target.level, - target.factions, + return new Character( + new CharacterState(target.name, newHealth, newStatus, target.level, target.factions), ); } } diff --git a/src/Health.ts b/src/Health.ts index 2a5cce6..6b6dd3d 100644 --- a/src/Health.ts +++ b/src/Health.ts @@ -11,10 +11,6 @@ export class Health { this.#value = n; } - static maxHealthForLevel(level: number): number { - return level >= 6 ? 1500 : 1000; - } - static create(n: number): Health { if (n < 0) { throw new Error(`Health cannot be negative, got ${n}`); diff --git a/src/damage-and-health.spec.ts b/src/damage-and-health.spec.ts index 9990273..6febdff 100644 --- a/src/damage-and-health.spec.ts +++ b/src/damage-and-health.spec.ts @@ -18,8 +18,8 @@ describe('DamageAndHealth', () => { health, }); const expected = Math.max(0, health - damage); - attacker.dealDamage(target, damage); - return target.health.value === expected; + const result = attacker.dealDamage(target, damage); + return result.health.value === expected; }, ), ); @@ -39,8 +39,8 @@ describe('DamageAndHealth', () => { level: Level.create(1), health, }); - attacker.dealDamage(target, damage); - return target.health.value >= 0; + const result = attacker.dealDamage(target, damage); + return result.health.value >= 0; }, ), ); @@ -60,12 +60,12 @@ describe('DamageAndHealth', () => { level: Level.create(1), health, }); - attacker.dealDamage(target, damage); + const result = attacker.dealDamage(target, damage); const expected = Math.max(0, health - damage); if (expected === 0) { - return target.status.kind === 'dead'; + return result.status.kind === 'dead'; } - return target.status.kind === 'alive'; + return result.status.kind === 'alive'; }, ), ); @@ -73,7 +73,7 @@ describe('DamageAndHealth', () => { }); describe('SelfDamageForbidden', () => { - it('property: a character cannot deal damage to itself — health unchanged', () => { + it('property: a character cannot deal damage to itself — returns same reference, state unchanged', () => { fc.assert( fc.property( fc.integer({ min: 0, max: 1000 }), @@ -82,8 +82,13 @@ describe('DamageAndHealth', () => { const c = Character.createWithHealth({ name: 'hero', level: Level.create(1), health }); const healthBefore = c.health.value; const statusBefore = c.status.kind; - c.dealDamage(c, damage); - return c.health.value === healthBefore && c.status.kind === statusBefore; + const result = c.dealDamage(c, damage); + // Should return the same reference + return ( + result === c && + result.health.value === healthBefore && + result.status.kind === statusBefore + ); }, ), ); @@ -91,18 +96,40 @@ describe('DamageAndHealth', () => { }); describe('DeadCannotTakeDamage', () => { - it('property: dead characters cannot take damage — state unchanged', () => { + it('property: dead characters cannot take damage — returns same reference, state unchanged', () => { fc.assert( fc.property(fc.integer({ min: 0, max: 10000 }), (damage) => { const attacker = Character.create({ name: 'attacker', level: Level.create(1) }); const target = Character.create({ name: 'target', level: Level.create(1) }); - // Kill the target first - attacker.dealDamage(target, 10000); - const healthBefore = target.health.value; - const statusBefore = target.status.kind; - // Then try to deal more damage - attacker.dealDamage(target, damage); - return target.health.value === healthBefore && target.status.kind === statusBefore; + // Kill the target first — capture the returned (dead) character + const deadTarget = attacker.dealDamage(target, 10000); + const healthBefore = deadTarget.health.value; + const statusBefore = deadTarget.status.kind; + // Then try to deal more damage to the dead character + const result = attacker.dealDamage(deadTarget, damage); + return ( + result === deadTarget && + result.health.value === healthBefore && + result.status.kind === statusBefore + ); + }), + ); + }); + }); + + describe('NegativeDamageForbidden', () => { + it('property: negative damage throws an error', () => { + fc.assert( + fc.property(fc.integer({ min: -10000, max: -1 }), (negativeDamage) => { + const attacker = Character.create({ name: 'attacker', level: Level.create(1) }); + const target = Character.create({ name: 'target', level: Level.create(1) }); + let threw = false; + try { + attacker.dealDamage(target, negativeDamage); + } catch { + threw = true; + } + return threw; }), ); });