Post Deleted by Community
    Post Locked by Community
    Post Migrated Away to softwareengineering.stackexchange.com by 200_success
    Post Closed as "off-topic" by 200_success
2 deleted 2 characters in body
source | link

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.

  • 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.

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.

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.
1
source | link

Having a side-effect free constructor while ensuring consistency

I recently stumbled upon this code where one wants to create a Path of Segments 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 Segments. 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;
        }
    }
}