Ideas/457 map table directly to constructor for stepargumenttransformers

[AroglDarthu]

🤔 What's changed?

A new option was added to change the behaviour of the table helper CreateInstance.
WIth the option set to true, the table helper will try to find a constructor that exactly matches the table.
If a matching constructor cannot be found, the helper will throw a MissingMethodException.

See https://github.com/orgs/reqnroll/discussions/457

⚡️ What's your motivation?

In normal circumstances you would not need this feature, because you can just give the support model the correct properties or fields to resemble the table. However, in my use-case I need the support model to be serialized into an exact JSON structure. For example:

{
  "MessageCreatedAt" : "2025-02-22T14:13:38+01:00",
  "MessageContentType" : "ProductCreatedMessageContent",
  "MessageContent" : {
    "ProductCode" : "X0010001B",
    "ProductName" : "Teddy Bear",
    "StartOfSale" : "2025-03-01"
  }
}

  | MessageCreatedAt          | ProductCode | ProductName | StartOfSale |
  | 2025-02-22T14:13:38+01:00 | X0010001B   | Teddy Bear  | 2025-03-01  |

Usage:

[StepArgumentTransformation]
public ProductCreatedMessage CreateInstance(Table table)
{
    var options = new InstanceCreationOptions
    {
        UseStrictTableToConstructorBinding = true
    };
    return table.CreateInstance<ProductCreatedMessage>(options);
}

🏷️ What kind of change is this?

• ⚡ New feature (non-breaking change which adds new behaviour)

📋 Checklist:

• I've changed the behaviour of the code
  • I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

---

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

[gasparnagy]

I like the concept... Just thinking about the naming. Here are my thoughts.

As far as I understand (from code and from your explanation), currently the implementation only passes table cells for the creation if there is a corresponding property. If a property is found and there is no default constructor, it invokes a constructor with a signature that corresponds to the properties, but the property is mandatory. (I.e. all constructor property has to have an associated property.)

As far as I understood your motivation, there are cases, where there is no explicit property for a value, but we would like to have it passed in through constructor.

The option UseStrictTableToConstructorBinding is a bit too generic, because it does not really tell what "strict" means in this context. I'm thinking in something like BindToConstructorParametersWithoutProperty or BindToConstructorParameters? What do you think?

For the implementation. You anyway have the tests, so you could play:

1. I think the current implementation (regardless of this PR) of the GetConstructorMatchingToColumnNames method is wrong as it only queries the properties although we can also set fields. I think the method should have used the GetMembersThatNeedToBeSet method to get the set of "members" instead of calling the GetProperties directly.
2. If that would be the case, maybe it would be enough to change (extend) only the GetMembersThatNeedToBeSet method to support your need: if the option is set, you should create MemberHandler instances for all constructor parameters in addition that does not have a corresponding field or property. (In this MemberHandler, the "set" delegate can throw exception, because it is never going to be set directly, just through the ctor. For this, you need to pass the InstanceCreationOptions to the GetMembersThatNeedToBeSet method, but this is not a problem as far as I see.

Could you try this approach if it works?

[AroglDarthu]

I like the concept... Just thinking about the naming. Here are my thoughts.

As far as I understand (from code and from your explanation), currently the implementation only passes table cells for the creation if there is a corresponding property. If a property is found and there is no default constructor, it invokes a constructor with a signature that corresponds to the properties, but the property is mandatory. (I.e. all constructor property has to have an associated property.)

The current mechanism is a bit worse even: not all constructor parameters need to have an associated property. It only looks like that way ;-)

Only the projected properties are taken into account. So properties with matching table columns. Then the first constructor is used that leads to an empty set of names when striking each parameter name out of the list of projected property names.

So in my example, only the MessageCreatedAt property gets projected, because it is the only property and also in the table. Then the first constructor with parameters containing this projected list is used. My constructor has three parameters, of which one is called messageCreatedAt. So this constructor oddly enough matches the requirements of GetConstructorMatchingToColumnNames.

As far as I understood your motivation, there are cases, where there is no explicit property for a value, but we would like to have it passed in through constructor.

Yeah, my take on this is that when your model does not have a default constructor, the table should provide values for all constructor parameters. Simply because the constructor is in charge of the initialization. It is a different use-case than what you normally need. By default you should (in my opinion) just create a model with a default constructor and initialize it through properties/fields.

The option UseStrictTableToConstructorBinding is a bit too generic, because it does not really tell what "strict" means in this context. I'm thinking in something like BindToConstructorParametersWithoutProperty or BindToConstructorParameters? What do you think?

Naming is always difficult ;-)
I like BindToConstructorParameters, but the current mechanism also does that in a way. Perhaps RequireAllConstructorParametersInTable?

For the implementation. You anyway have the tests, so you could play:

1. I think the current implementation (regardless of this PR) of the GetConstructorMatchingToColumnNames method is wrong as it only queries the properties although we can also set fields. I think the method should have used the GetMembersThatNeedToBeSet method to get the set of "members" instead of calling the GetProperties directly.

I agree.

2. If that would be the case, maybe it would be enough to change (extend) only the GetMembersThatNeedToBeSet method to support your need: if the option is set, you should create MemberHandler instances for all constructor parameters in addition that does not have a corresponding field or property. (In this MemberHandler, the "set" delegate can throw exception, because it is never going to be set directly, just through the ctor. For this, you need to pass the InstanceCreationOptions to the GetMembersThatNeedToBeSet method, but this is not a problem as far as I see.

I see where you want to go. I deliberately separated the two approaches to ensure not changing the current mechanism and to not introduce any breaking changes. That, and I wanted to force a single method of initialization. But I can see the flexibility in your approach.

Could you try this approach if it works?

Sure!
💥 Well, looking over the code again, there is a challenge when extending GetMembersThatNeedToBeSet: how should the option VerifyAllColumnsBound behave?

[Code-Grump]

I'm curious: do we think that an ASP.NET-like "model binding" is what could be most useful/idiomatic here?

Something which tries to take the input values provided by the table row and construct a type from them:

• Using matching constructor arguments where a suitable constructor can be found.
• Invoking property setters when not already matched by the constructor.
• Ensuring all values from the table row were consumed / the result model is valid.

It's a similar problem space, particularly with how the HTTP payload and the table row are both text-based needing to be converted into strongly-typed models. It also opens the door to more advanced compositions in the future, such as including values from additional sources.

[gasparnagy]

I'm curious: do we think that an ASP.NET-like "model binding" is what could be most useful/idiomatic here?

Yes, that sounds useful, but that one is a more complex change. @AroglDarthu What do you think?

[gasparnagy]

Well, looking over the code again, there is a challenge when extending GetMembersThatNeedToBeSet: how should the option VerifyAllColumnsBound behave?

I think with this option, this setting has no relevance (because you anyway need to bind all ctor params to call the ctor), so I would ignore it.

[AroglDarthu]

I'm curious: do we think that an ASP.NET-like "model binding" is what could be most useful/idiomatic here?

That does make a lot of sense. I think the binding mechanism would feel more intuitive that way. Not sure if the current static helpers are the way to go, as it will probably cause breaking changes or at least different behavior.

Perhaps it could fit in nicely with the DI route @ajeckmans is aiming for in #108. It could (as fas as I understood) be introduced as a new feature alltogether, while still supporting the current static helpers. The static helpers can then be marked as deprecated with a message hinting at the new feature.

[AroglDarthu]

My last refactoring does pretty much what @Code-Grump suggested:

• Using matching constructor arguments where a suitable constructor can be found.
• Invoking property setters when not already matched by the constructor.
• Ensuring all values from the table row were consumed / the result model is valid.

Without any breaking changes (at least that was my objective)...

1. If there is a default constructor, that will take precedence;
2. If not explicitly set to RequireTableToProvideAllConstructorParameters, then the current constructor resolution is used - meaning it will try to find a constructor that takes all provided members (now including fields) and it allows default parameter values;
3. Setting RequireTableToProvideAllConstructorParameters to true will cause the constructor resolution to find one that matches all columns from the table. To do that it first tries to get a match with all provided members. If it is still missing parameters, then columns not explicitly bound to a member will be probed. Once a match is found, the resolved extra columns are added to the list of member handlers;
4. After invoking the constructor, any member handlers that were not used to construct the instance will then be used to set additional members;
5. Verification of bound columns works on all added member handlers, so also the ones without members (based on constructor parameters)