SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
781 stars 45 forks source link

Factory support #543

Closed AthenaAzuraeaX closed 1 month ago

AthenaAzuraeaX commented 9 months ago

Describe the feature

Hi,

Let me just first complement you on a nice library for value objects.

Now, I would like to suggest the possibility to support Factories to construct the value objects instead of the static From method.

Consider this code:

[DebuggerStepThrough]
readonly record struct IntegralRange(int Min, int Max)
{
    public IntegralRange Narrowest(int Min, int Max) => new(Math.Max(this.Min, Min), Math.Min(this.Max, Max));
    public override String ToString() => throw new NotSupportedException();
}

[ValueObject(typeof(Int32))]
internal sealed partial class Port
{
    private const Int32 _Min = 1100;
    private const Int32 _Max = 49000;

    public static OneOf<IValidationResultCollection, Port> TryFrom(Int32? Value) =>
        TryFrom(new(_Min, _Min), Value);

    public static OneOf<IValidationResultCollection, Port> TryFrom(IntegralRange Range, Int32? Value)
    {
        var Validation = ValidateImpl(Range.Narrowest(_Min, _Max), Value);
        return Validation.Success
            ? new Port(Value!.Value)
            : OneOf<IValidationResultCollection, Port>.FromT0(Validation);
    }

    private static Vogen.Validation Validate(Int32 Input)
    {
        var Validation = ValidateImpl(new(_Min, _Min), Input);
        return Validation.Success ? Vogen.Validation.Ok : Vogen.Validation.Invalid(Validation.GetErrors().JoinString("\n"));
    }

    private static IValidationResultCollection ValidateImpl(IntegralRange Range, Int32? Value)
    {
        var Model = ValidationModelFactory<Int32>.Create().RangeInclusive(Range.Min, Range.Max, Unit: null).Build();
        return new ModelValidator().Validate(Model, Value);
    }
}

Here we can manually specify a narrower allowed Range for Port depending on our domain rules, by always ensuring the Port is within acceptable good practices. But the From method makes this custom range difficult to use. Due to its static nature, it would need the custom range to be static or somehow dependency injected via some framework.

That is why I would like to propose the Factory pattern, where we can instantiate a Factory, whose Create or Build method can create an instance of the Value object. Since the Factory is an instance, we can inject variables into it necessary for the constructing the value object. For example:

[ValueObject(typeof(Int32))]
internal sealed partial class Port
{
    public sealed class Factory
    {
        private const Int32 _Min = 1100;
        private const Int32 _Max = 49000;
        private readonly IntegralRange _Range;

        public Factory() => _Range = new(_Min, _Max);
        public Factory(IntegralRange Range) => _Range = Range.Narrowest(_Min, _Max);
        public Port From(Int32? Value) => TryFrom(Value).Value as Port ?? throw new Exception("My message");

        public OneOf<IValidationResultCollection, Port> TryFrom(Int32? Value)
        {
            var Validation = ValidateImpl(_Range, Value);
            return Validation.Success
                ? new Port(Value!.Value)
                : OneOf<IValidationResultCollection, Port>.FromT0(Validation);
        }

        private static IValidationResultCollection ValidateImpl(IntegralRange Range, Int32? Value)
        {
            var Model = ValidationModelFactory<Int32>.Create().RangeInclusive(Range.Min, Range.Max, Unit: null).Build();
            return new ModelValidator().Validate(Model, Value);
        }
    }
}

And now we can create it using:

var Port = new Port.Factory(new(2000, 2100)).TryFrom(1200);

Ensuring consistency between the TryFrom (would love to see a TryFrom method, btw) and From.

This could perhaps be optionally opted-into using an attribute. Ideally, I'd love to see similar approaches for serialization too, to keep things consistent.

SteveDunn commented 9 months ago

Hi, thanks for the feedback and suggestion, much appreciated!

First impressions look like it'd be quite a major change, but I can see how such a feature would be useful thanks to your example.

AthenaAzuraeaX commented 9 months ago

I don't know what's up with Github not formatting code correctly. Not sure how to do it, but great that you could still understand my example!

If you're up for maintaining this feature going forward, then I figure I might try to contribute a little. But Vogen is a big project. I don't know where I would start, honestly. But having this feature would be a boon for me, and really add a lot of flexibility to value objects, I think.

SteveDunn commented 4 months ago

This would be quite a big feature to implement. I think maybe the first step would be to consider adding just the TryFrom method.

Then, maybe in the future, a feature closer to what you describe could be implemented outside of Vogen by using static abstract interfaces.

AthenaAzuraeaX commented 4 months ago

A TryFrom method would absolutely be a boon. But I also think that a bigger boon would be the ability to remove static methods like From and TryFrom and either generate a factory type for constructing the type or use normal constructors. The problem with static methods is that they can't be mocked and are therefore difficult to test.

SteveDunn commented 1 month ago

Hi @AthenaAzuraeaX - it is now possible to create factories. The documentation describes how to use it for sequential GUIDs, but the concept is the same for your case.

AthenaAzuraeaX commented 1 month ago

Was what was added only the interface so we could do a .From on a type parameter, as in the example in the documentation? My idea was more to eliminate the static From and TryFrom methods and instead use a (non-static) Factory to create instances of the value object. This allows for much more flexibility, such as passing down Factories using interfaces and injecting state into the factories. Not to mention, much more testable since static methods aren't mockable.

SteveDunn commented 1 month ago

@AthenaAzuraeaX - From is still present in the value objects. The factory, although static in the example, can be constructed any way you like and injected with interfaces of other types.

more testable since static methods aren't mockable.

For typical value objects, I've never needed to mock them. Could you describe a scenario where mocking would have been useful?

AthenaAzuraeaX commented 1 month ago

Well, one situation where one might want to mock value objects is when one specifically want to be able to create invalid units for something like testing how a system might recover from invalid inputs. But I don't think the question is about why - but rather about what advantages using non-static methods give.