Refactor World to avoid double enumeration

If an enumerable that created objects were passed, it would duplicate the objects when re-enumerated, which breaks all the reference equality logic.

MultiversalDiplomacy/Model/World.cs

@@ -1,3 +1,5 @@
+using System.Collections.ObjectModel;
+
 namespace MultiversalDiplomacy.Model;
 
 /// <summary>
@@ -8,33 +10,33 @@ public class World
     /// <summary>
     /// The game map.
     /// </summary>
-    public IEnumerable<Province> Provinces { get; }
+    public ReadOnlyCollection<Province> Provinces { get; }
 
     /// <summary>
     /// The game powers.
     /// </summary>
-    public IEnumerable<Power> Powers { get; }
+    public ReadOnlyCollection<Power> Powers { get; }
 
     /// <summary>
     /// The state of the multiverse.
     /// </summary>
-    public IEnumerable<Season> Seasons { get; }
+    public ReadOnlyCollection<Season> Seasons { get; }
 
     /// <summary>
     /// All units in the multiverse.
     /// </summary>
-    public IEnumerable<Unit> Units { get; }
+    public ReadOnlyCollection<Unit> Units { get; }
 
     /// <summary>
     /// Immutable game options.
     /// </summary>
     public Options Options { get; }
 
-    public World(
+    private World(
-        IEnumerable<Province> provinces,
+        ReadOnlyCollection<Province> provinces,
-        IEnumerable<Power> powers,
+        ReadOnlyCollection<Power> powers,
-        IEnumerable<Season> seasons,
+        ReadOnlyCollection<Season> seasons,
-        IEnumerable<Unit> units,
+        ReadOnlyCollection<Unit> units,
         Options options)
     {
         this.Provinces = provinces;
@@ -45,21 +47,105 @@ public class World
     }
 
     /// <summary>
-    /// Create a new world with no map, powers, or units, and a root season.
+    /// Create a new world with specified provinces and powers.
     /// </summary>
-    public static World Empty => new World(
+    public static World WithMap(IEnumerable<Province> provinces, IEnumerable<Power> powers)
-        new List<Province>(),
+        => new World(
-        new List<Power>(),
+            new(provinces.ToList()),
-        new List<Season> { Season.MakeRoot() },
+            new(powers.ToList()),
-        new List<Unit>(),
+            new(new List<Season>()),
+            new(new List<Unit>()),
             new Options());
 
     /// <summary>
-    /// Create a world with a standard map, powers, and initial unit placements.
+    /// Create a new world with the standard Diplomacy provinces and powers.
     /// </summary>
-    public static World Standard => Empty
+    public static World WithStandardMap()
+        => WithMap(StandardProvinces, StandardPowers);
+
+    /// <summary>
+    /// Create a new world with new seasons.
+    /// </summary>
+    public World WithSeasons(IEnumerable<Season> seasons)
+        => new World(this.Provinces, this.Powers, new(seasons.ToList()), this.Units, this.Options);
+
+    /// <summary>
+    /// Create a new world with an initial season.
+    /// </summary>
+    public World WithInitialSeason()
+        => WithSeasons(new List<Season> { Season.MakeRoot() });
+
+    /// <summary>
+    /// Create a new world with new units.
+    /// </summary>
+    public World WithUnits(IEnumerable<Unit> units)
+        => new World(this.Provinces, this.Powers, this.Seasons, new(units.ToList()), this.Options);
+
+    /// <summary>
+    /// Create a new world with new units created from unit specs. Units specs are in the format
+    /// "<power> <A/F> <province> [<coast>]". If the province or coast name has a space in it, the
+    /// abbreviation should be used.
+    /// </summary>
+    public World WithUnits(params string[] unitSpecs)
+    {
+        IEnumerable<Unit> units = unitSpecs.Select(spec =>
+        {
+            string[] splits = spec.Split(' ', 4);
+            Power power = this.GetPower(splits[0]);
+            UnitType type = splits[1] switch
+            {
+                "A" => UnitType.Army,
+                "F" => UnitType.Fleet,
+                _ => throw new ArgumentOutOfRangeException($"Unknown unit type {splits[1]}")
+            };
+            Location location = type == UnitType.Army
+                ? this.GetLand(splits[2])
+                : splits.Length == 3
+                    ? this.GetWater(splits[2])
+                    : this.GetWater(splits[2], splits[3]);
+            Unit unit = Unit.Build(location, this.Seasons.First(), power, type);
+            return unit;
+        });
+        return this.WithUnits(units);
+    }
+
+    /// <summary>
+    /// Create a new world with standard Diplomacy initial unit placements.
+    /// </summary>
+    public World WithStandardUnits()
+    {
+        return this.WithUnits(
+            "Austria A Bud",
+            "Austria A Vir",
+            "Austria F Tri",
+            "England A Lvp",
+            "England F Edi",
+            "England F Lon",
+            "France A Mar",
+            "France A Par",
+            "France F Bre",
+            "Germany A Ber",
+            "Germany A Mun",
+            "Germany F Kie",
+            "Italy A Rom",
+            "Italy A Ven",
+            "Italy F Nap",
+            "Russia A Mos",
+            "Russia A War",
+            "Russia F Sev",
+            "Russia F Stp wc",
+            "Turkey A Con",
+            "Turkey A Smy",
+            "Turkey F Ank"
+        );
+    }
+
+    /// <summary>
+    /// A standard Diplomacy game setup.
+    /// </summary>
+    public static World Standard => World
         .WithStandardMap()
-        .WithStandardPowers()
+        .WithInitialSeason()
         .WithStandardUnits();
 
     /// <summary>
@@ -68,7 +154,7 @@ public class World
     private Province GetProvince(string provinceName)
     {
         string provinceNameUpper = provinceName.ToUpperInvariant();
-        Province? foundProvince = this.Provinces.FirstOrDefault(
+        Province? foundProvince = this.Provinces.SingleOrDefault(
             p => p != null &&
                 (p.Name.ToUpperInvariant() == provinceNameUpper
                 || p.Abbreviations.Any(a => a.ToUpperInvariant() == provinceNameUpper)),
@@ -106,11 +192,11 @@ public class World
             : GetLocation(provinceName, l => l.Name == coastName || l.Abbreviation == coastName);
 
     /// <summary>
-    /// Get a power by name. Throws if the power is not found.
+    /// Get a power by name. Throws if there is not exactly one such power.
     /// </summary>
     public Power GetPower(string powerName)
     {
-        Power? foundPower = this.Powers.FirstOrDefault(
+        Power? foundPower = this.Powers.SingleOrDefault(
             p =>
                 p != null
                 && (p.Name == powerName || p.Name.StartsWith(powerName)),
@@ -121,19 +207,23 @@ public class World
     }
 
     /// <summary>
-    /// Create a new world from this one with new provinces.
+    /// Returns a unit in a province. Throws if there is not exactly one such unit.
     /// </summary>
-    public World WithMap(IEnumerable<Province> provinces)
+    public Unit? GetUnitAt(string provinceName)
     {
-        if (this.Units.Any()) throw new InvalidOperationException(
+        Province province = GetProvince(provinceName);
-            "Provinces cannot be changed once units have been placed on the map");
+        Unit? foundUnit = this.Units.SingleOrDefault(
-        return new World(provinces, this.Powers, this.Seasons, this.Units, this.Options);
+            u => u != null && u.Location.Province == province,
+            null);
+        return foundUnit;
     }
 
     /// <summary>
-    /// Create a new world from this one with the standard Diplomacy provinces.
+    /// The standard Diplomacy provinces.
     /// </summary>
-    public World WithStandardMap()
+    public static ReadOnlyCollection<Province> StandardProvinces
+    {
+        get
         {
             // Define the provinces of the standard world map.
             List<Province> standardProvinces = new List<Province>
@@ -440,97 +530,24 @@ public class World
 
             // TODO
 
-        return this.WithMap(standardProvinces);
+            return new(standardProvinces);
+        }
     }
 
     /// <summary>
-    /// Create a new world from this one with new powers.
+    /// The standard Diplomacy powers.
     /// </summary>
-    public World WithPowers(IEnumerable<Power> powers)
+    public static ReadOnlyCollection<Power> StandardPowers
-        => new World(this.Provinces, powers, this.Seasons, this.Units, this.Options);
-
-    /// <summary>
-    /// Create a new world from this one with new powers created with the given names.
-    /// </summary>
-    public World WithPowers(IEnumerable<string> powerNames)
-        => WithPowers(powerNames.Select(name => new Model.Power(name)));
-
-    /// <summary>
-    /// Create a new world from this one with new powers created with the given names.
-    /// </summary>
-    public World WithPowers(params string[] powerNames)
-        => WithPowers(powerNames.AsEnumerable());
-
-    /// <summary>
-    /// Create a new world from this one with the standard Diplomacy powers.
-    /// </summary>
-    public World WithStandardPowers()
-        => WithPowers("Austria", "England", "France", "Germany", "Italy", "Russia", "Turkey");
-
-    /// <summary>
-    /// Create a new world from this one with new units created from unit specs. Units specs are
-    /// in the format "<power> <A/F> <province> [<coast>]". If the province or coast name has a
-    /// space in it, the abbreviation should be used.
-    /// </summary>
-    public World WithUnits(IEnumerable<string> unitSpec)
     {
-        IEnumerable<Unit> units = unitSpec.Select(spec =>
+        get => new(new List<Power>
         {
-            string[] splits = spec.Split(' ', 4);
+            new Power("Austria"),
-            Power power = this.GetPower(splits[0]);
+            new Power("England"),
-            UnitType type = splits[1] switch
+            new Power("France"),
-            {
+            new Power("Germany"),
-                "A" => UnitType.Army,
+            new Power("Italy"),
-                "F" => UnitType.Fleet,
+            new Power("Russia"),
-                _ => throw new ArgumentOutOfRangeException($"Unknown unit type {splits[1]}")
+            new Power("Turkey"),
-            };
-            Location location = type == UnitType.Army
-                ? this.GetLand(splits[2])
-                : splits.Length == 3
-                    ? this.GetWater(splits[2])
-                    : this.GetWater(splits[2], splits[3]);
-            Unit unit = Unit.Build(location, this.Seasons.First(), power, type);
-            return unit;
         });
-        return new World(this.Provinces, this.Powers, this.Seasons, units, this.Options);
-    }
-
-    /// <summary>
-    /// Create a new world from this one with new units created from unit specs. Units specs are
-    /// in the format "<power> <A/F> <province> [<coast>]".
-    /// </summary>
-    public World WithUnits(params string[] unitSpec)
-        => this.WithUnits(unitSpec.AsEnumerable());
-
-    /// <summary>
-    /// Create a new world from this one with new units created according to the standard Diplomacy
-    /// initial unit deployments.
-    /// </summary>
-    public World WithStandardUnits()
-    {
-        return this.WithUnits(
-            "Austria A Bud",
-            "Austria A Vir",
-            "Austria F Tri",
-            "England A Lvp",
-            "England F Edi",
-            "England F Lon",
-            "France A Mar",
-            "France A Par",
-            "France F Bre",
-            "Germany A Ber",
-            "Germany A Mun",
-            "Germany F Kie",
-            "Italy A Rom",
-            "Italy A Ven",
-            "Italy F Nap",
-            "Russia A Mos",
-            "Russia A War",
-            "Russia F Sev",
-            "Russia F Stp wc",
-            "Turkey A Con",
-            "Turkey A Smy",
-            "Turkey F Ank"
-        );
     }
 }

MultiversalDiplomacyTests/AdjudicatorTests.cs

@@ -15,8 +15,8 @@ public class AdjudicatorTests
         {
             return orders.Select(o => o.Validate(ValidationReason.Valid)).ToList();
         });
-        World world = World.Empty.WithPowers("Power");
+        World world = World.WithStandardMap().WithInitialSeason();
-        Power power = world.GetPower("Power");
+        Power power = world.GetPower("Austria");
 
         Order order = new NullOrder(power);
         List<Order> orders = new List<Order> { order };

MultiversalDiplomacyTests/MapTests.cs

@@ -50,7 +50,7 @@ public class MapTests
     [Test]
     public void LandAndSeaBorders()
     {
-        World map = World.Empty.WithStandardMap();
+        World map = World.WithStandardMap();
         Assert.That(
             map.GetLand("NAF").Adjacents.Count(),
             Is.EqualTo(1),

MultiversalDiplomacyTests/UnitTests.cs

@@ -9,9 +9,11 @@ public class UnitTests
     [Test]
     public void MovementTest()
     {
-        World world = World.Empty.WithStandardMap().WithPowers("First");
+        World world = World.WithStandardMap().WithInitialSeason();
-        Location Mun = world.GetLand("Mun"), Boh = world.GetLand("Boh"), Tyr = world.GetLand("Tyr");
+        Location Mun = world.GetLand("Mun"),
-        Power pw1 = world.GetPower("First");
+            Boh = world.GetLand("Boh"),
+            Tyr = world.GetLand("Tyr");
+        Power pw1 = world.GetPower("Austria");
         Season s1 = world.Seasons.First();
         Unit u1 = Unit.Build(Mun, s1, pw1, UnitType.Army);