0

I have a very complex type. The type has multiple readonly properties and each property is a complex type:

public class FinalResult
{
    public ComplexType1 ComplexType1 {get;set;}

    public ComplexType2 ComplexType2 {get;set;}

    public ComplexType3 ComplexType3 {get;set;}

    public ComplexType4 ComplexType4 {get;set;}
}

Because ComplextTypeX is complex I have created a builder for each of them:

public class ComplexType1Builder
{
    public ComplexType1 Build(Dependency dep, OtherDependency otherDep)
    {
        // build / create the complex object

        return complexType1;
    }
}

public class ComplexType2Builder
{
    public ComplexType2 Build(Dependency dep, OtherDependency otherDep)
    {
        // build / create the complex object

        return complexType2;
    }
}

// same for ComplexType3 and ComplexType4

Finally I have FinalResultDirector which ties everything together:

public class FinalResultDirector
{

    private readonly ComplexType1Builder _builder1;
    private readonly ComplexType2Builder _builder2;
    private readonly ComplexType3Builder _builder3;
    private readonly ComplexType4Builder _builder4;

    public FinalResultDirector(
        ComplexType1Builder builder1, 
        ComplexType2Builder builder2, 
        ComplexType3Builder builder3, 
        ComplexType4Builder builder4 )
    {
        _builder1 = builder1;
        _builder2 - builder2;
        _builder3 = builder3;
        _builder4 = builder4;
    }

    public FinalResult Build(Dependency dep, OtherDependency otherDep)
    {
        return new FinalResult
        {
            ComplexType1 => _builder1.Build(dep, otherDep),

            ComplexType2 => _builder2.Build(dep, otherDep),

            ComplexType3 => _builder3.Build(dep, otherDep),

            ComplexType4 => _builder4.Build(dep, otherDep),
        };
    }
}

This works, but my OO heart is crying. I know this can be solved much nicer, cleaner and more SOLID, but I haven't seen the solution.

I've tried using interfaces, but since each builder returns a different type, it doesn't have any additional value.

Another approach I've been thinking about is new up a FinalResult object and have each Build method accept this as a third parameter: public ComplexTypeX Build(Dependency dep, OtherDependency otherDep, FinalResult finalResult). And then return void. Now all the builders have the same method signature. The downside of this approach is that you get tempted to use properties on FinalResult that aren't set yet.

I'd love to hear how to solve this in a nice OO way.

Martijn
  • 24,441
  • 60
  • 174
  • 261
  • Why do you need such complicated `FinalResult` class, with complex types inside? Have you thought about decoupling it? Does it always has all complex types all the time or it depends on some logic? Otherwise you may look at fluent builder – Pavel Anikhouski May 07 '20 at 09:13
  • `FinalResult` won't compile, because properties must have a names, not the type only – Pavel Anikhouski May 07 '20 at 09:24
  • Fixed the code. Yes, as far as I can see at the moment, I need al these complex types. – Martijn May 07 '20 at 09:25
  • Properties setters still not accessible, all builders has the same `ComplexType1Builder` type. Please, post your actual code – Pavel Anikhouski May 07 '20 at 09:28
  • This is a real representation of my code. I can't post the real code because it contains confidential stuff. – Martijn May 07 '20 at 09:34
  • 1
    Well, you've passing 4 builders with different types to director class, but assign them to 4 variables with the same type `ComplexType1Builder`. Why do you need 4 builders then? This line will not compile `_builder2 - builder2;` Did you try to build your sample at least? It can't be a real code, because it'll never run – Pavel Anikhouski May 07 '20 at 09:38
  • 1
    If you have code that works but could be better (possibly not this code) [Code Review](https://codereview.stackexchange.com) is a better match. Do make sure you have actual working code before posting there, though. – Jeroen Mostert May 07 '20 at 09:41
  • You should assume builders need different dependencies as well as shared ones. It will help you thinking of a design that can accommodate such evolution. – plalx May 15 '20 at 03:10

1 Answers1

0

You could use a single Builder class that builds all types. The general rule (to me) is that if you have lots of classes that each have only 1 method, then that is overdesigned and you should at least consider making fewer classes or even a single class with all those methods.

Now that we have one Builder class, it also becomes easier to try eliminating the FinalResultDirector class.

This + I added internal set; because you can't assign readonly properties except from the constructor or from an initializer inside the class.

Working code + dotnetfiddle:

using System;

public class Program
{
    public static void Main()
    {
        var director = new FinalResultDirector(new ComplexTypeBuilder());
        var result = director.Build(new Dependency(), new OtherDependency());

        Console.WriteLine(result);
    }
}

public class ComplexType1 {}
public class ComplexType2 {}
public class ComplexType3 {}
public class ComplexType4 {}

public class Dependency {}
public class OtherDependency{}

public class FinalResult
{
    public ComplexType1 ComplexType1 {get; internal set;}
    public ComplexType2 ComplexType2 {get; internal set;}
    public ComplexType3 ComplexType3 {get; internal set;}
    public ComplexType4 ComplexType4 {get; internal set;}
}

public class ComplexTypeBuilder
{
    public ComplexType1 Build1(Dependency dep, OtherDependency otherDep)
    {
        return new ComplexType1();
    }
    public ComplexType2 Build2(Dependency dep, OtherDependency otherDep)
    {
        return new ComplexType2();
    }
    public ComplexType3 Build3(Dependency dep, OtherDependency otherDep)
    {
        return new ComplexType3();
    }
    public ComplexType4 Build4(Dependency dep, OtherDependency otherDep)
    {
        return new ComplexType4();
    }
}

public class FinalResultDirector
{
    private readonly ComplexTypeBuilder _builder;

    public FinalResultDirector(ComplexTypeBuilder builder)
    {
        _builder = builder;
    }

    public FinalResult Build(Dependency dep, OtherDependency otherDep)
    {
        return new FinalResult
        {
            ComplexType1 = _builder.Build1(dep, otherDep),
            ComplexType2 = _builder.Build2(dep, otherDep),
            ComplexType3 = _builder.Build3(dep, otherDep),
            ComplexType4 = _builder.Build4(dep, otherDep)
        };
    }
}
Peter B
  • 22,460
  • 5
  • 32
  • 69