I recently stumbled upon this code where one wants to create a Path
of Segment
s and a Segment
must be aware of which Path
it is on.
public class Path { public List<Segment> Segments { get; } = new List<Segment>(); } public class Segment { public int Property { get; } public Path Path { get; } ... } public static class Test { public static Path CreatePath() { var path = new Path(); var segment = new Segment(path, 42); return path; } }
and without inspecting further, I concluded that there clearly was a bug, since segment
wasn’t added to path.Segments
.
Apparently I was wrong as I expected a side-effect free constructor of Segment
. The actual constructor looked like this:
public Segment(Path path, int property) { Property = property; Path = path; path.Segments.Add(this); // <-- }
While I acknowledge the intend of the constructor to ensure that a Segment
is always added to the Segments
, I don’t like the way it’s done mainly of two reasons:
- The constructor does more than validating arguments and initializing field
- The constructor has side-effects
CreatePath
only looks correct if you know the side-effect of the constructor.
I tried to come up with alternative ways to accomplish this, but I’m not entirely satisfied with any of them. Maybe I’m missing out on a well-known design pattern?
The original code where the consistency is ensured inside the constructor of Segment
. It’s problem is, that segment
looks unused in CreatePath
if you don’t know about the internals of the constructor.
namespace PathExample0 { public class Path { public List<Segment> Segments { get; } = new List<Segment>(); } public class Segment { public int Property { get; } public Path Path { get; } public Segment(Path path, int property) { Property = property; Path = path; path.Segments.Add(this); } } public static class Test { public static Path CreatePath() { var path = new Path(); var segment = new Segment(path, 42); return path; } } }
This first alternative takes out the consistency part of the constructor and into the CreatePath
. The constructor is now free of side-effects and segment
is clearly used in CreatePath
. The minor problem is now, that were are not guaranteed that segment
is added to Segments
of the right Path
.
namespace PathExample1 { public class Path { public List<Segment> Segments { get; } = new List<Segment>(); } public class Segment { public int Property { get; } public Path Path { get; } public Segment(Path path, int property) { Property = property; Path = path; } } public static class Test { public static Path CreatePath() { var path = new Path(); var segment = new Segment(path, 42); path.Segments.Add(segment); return path; } } }
My second attempt Adds an AddSegment
to Path
which ensures the consistency. This still don’t hinders you to call new Segment(somePath, 42)
and break the consistency.
namespace PathExample2 { public class Path { private readonly List<Segment> segments = new List<Segment>(); public IReadOnlyList<Segment> Segments => segments; public void AddSegment(int property) { var segment = new Segment(this, property); segments.Add(segment); } } public class Segment { public int Property { get; } public Path Path { get; } public Segment(Path path, int property) { Property = property; Path = path; } } public static class Test { public static Path CreatePath() { var path = new Path(); path.AddSegment(42); return path; } } }
The third and last (and perhaps best) example I could come up with interfaces out Segment
into ISegment
and makes Segment
a private class to Path
. This should now ensure full consistency between a Path
and its Segment
s. To me it feel like a cumbersome approach and almost as a misuse of interfaces.
namespace PathExample3 { public interface ISegment { Path Path { get; } int Property { get; } } public class Path { private readonly List<Segment> segments = new List<Segment>(); public IReadOnlyList<ISegment> Segments => segments; public void AddSegment(int property) { var segment = new Segment(this, property); segments.Add(segment); } private class Segment : ISegment { public int Property { get; } public Path Path { get; } public Segment(Path path, int property) { Property = property; Path = path; } } } public static class Test { public static Path CreatePath() { var path = new Path(); path.AddSegment(42); return path; } } }