m31coding / M31.FluentAPI

Generate fluent builders for your C# classes with ease.
MIT License
94 stars 4 forks source link

Feature: Allow skipping stages #10

Closed vzam closed 3 months ago

vzam commented 4 months ago

I see some great use cases for the library in unit testing, to create some fake objects and convert them into my models. It would be nice if steps were skippable, such that the long lists such as:

CreateFakeStudent
  .WithRandomId()
  .WithRandomName()
  .WithRandomPhone()
  .WithBudget(100)

could be replaced with: CreateFakeStudent.WithBudget(100). This makes the intent of the faked object clearer. The WithRandomX methods I have shown are FluentDefaults so they would be implicit. A possible implementation would allow doing that:

[FluentApi]
class FakeStudent {
  [FluentMember(0)]
  [FluentSkippable]
  public Guid Id {get;set;} = Guid.NewGuid();

  [FluentMember(1)]
  [FluentSkippable]
  public string Name {get;set;} = "John Doe"; // not random but you get the point

  [FluentMember(2)]
  [FluentSkippable]
  public string Phome {get;set;} = "123";

  [FluentMember(3)]
  public int Budget {get;set;}
}

and generate:

// no interface for Id because it is the first step

interface INameSkippable : IPhoneSkippable {
  void WithName();
}

interface IPhoneSkippable : IBudget {
  void WithPhone();
}

interface IBudget {
  FakeStudent WithBudget();
}

static class CreateFakeStudent {
  static INameSkippable WithId() { ... }
  static IPhoneSkippable WithPhone() { ... }
  static IBudget WithBudget() { ... }
}
m31coding commented 4 months ago

Hi, thank you very much for this suggestion, I think a FluentSkippable attribute would be a great addition to the library. The meaning of the attribute is easy to grasp and the implementation straight forward. However, I noticed the following design issue for several members or methods at the same step (fork):

[FluentApi]
class Student
{
    [FluentMethod(0)]
    [FluentSkippable]
    public void MethodA()
    {
    }

    [FluentMethod(0)]
    public void MethodB()
    {
    }

    [FluentMethod(1)]
    public void MethodC()
    {
    }
}

The FluentSkippable attribute is specified at MethodA but MethodB would be skippable as well. Logically, the FluentSkippable attribute belongs to the step and not to individual members or methods of the step.

Hence, it could be better to specify it at the class level like so:

[FluentApi]
[FluentStep(0, Step.Skippable)]
class Student
{
    ...
}

The disadvantage of this approach is that it might be less intuitive if there is only one member or method at the skippable step. For this case your suggested syntax would be better.

I am happy for further discussion and ideas on this design issue.

vzam commented 4 months ago

Yes I didn’t think of that but your solution seems fine to me. That’s better than specifying the attribute for each method if you would otherwise throw errors on invalid combinations. Also I think that you could significantly simplify the solution by having step 0 also be defined through interfaces, e.g. instead of CreateStudent for the Student FluentAPI you would use StudentBuilder.Create, which would then return the interface for step 0 instead of having step 0 be static on the CreateStudent class.

vzam commented 4 months ago

Also if you would return an object for even the first step, it would allow to use the options pattern easier:

void DoSomething(Action<IStudentBuilder> builder) {
  IStudentBuilder studentBuilder = StudentBuilder.Create;
  Student student = builder(studentBuilder);

  // do something with student
}

DoSomething(builder => builder.WithFirstName("Santa").WithSurname("Clause"));
m31coding commented 3 months ago

Also if you would return an object for even the first step, it would allow to use the options pattern easier:

void DoSomething(Action<IStudentBuilder> builder) {
  IStudentBuilder studentBuilder = StudentBuilder.Create;
  Student student = builder(studentBuilder);

  // do something with student
}

DoSomething(builder => builder.WithFirstName("Santa").WithSurname("Clause"));

Thank you for this input, I am currently working on a feature that requires exactly what you describe here, see #14. I plan to keep the static methods for the first step and expose a new method InitialStep which returns the desired builder object.

m31coding commented 3 months ago

I have thought more about the FluentSkippable attribute in the context of forks, and I believe it is correct to annotate individual members and methods. Consider a builder with the following interface:

public interface IStep0
{
    IStep1 Method1();
    IStep2 Method2();
}

If, in the corresponding Fluent API class, Method1 is annotated with FluentSkippable the interface should become:

public interface IStep0 : IStep1
{
    IStep1 Method1();
    IStep2 Method2();
}

If both methods are annotated with FluentSkippable the interface will be:

public interface IStep0 : IStep1, IStep2
{
    IStep1 Method1();
    IStep2 Method2();
}

Consequently, if the interface is

public interface IStep0 
{
    IStep1 Method1();
    IStep1 Method2();
}

and one of the two methods is annotated with FluentSkippable, both methods will be skippable:

public interface IStep0 : IStep1
{
    IStep1 Method1();
    IStep1 Method2();
}