Closed antross closed 5 months ago
Let me think about this for a while. Sounds kinda right to possibly pass all of the same class to some kind of "DescribeEx'ish" function that takes a list of constraints that you can examine. Maybe just pass all the rule's constraints and let the called code sort stuff out. The Points code could scan looking for all ShowsPoints classes and just do the right thing. Another idea would be to do that work before calling a constraint. Something like DescribeMultiple() which would be called on a single instance, but would provide a list of all instances of the same class. I'm sure there's a way to do this using Reflection. So the code in Points would be something like
Describe(call, ps, List
which are all of the same class as ShowsPoints and it would be called a single time per class. Ideally it would be more like a templated interface, but this seems very fancy. I'm sure with Reflection we could do it for something like:
Describe\<ShowsPoints>(Call call, PositionState ps, IEnumerable\<ShowsPoints> constraints)
Maybe GitHub copilot will show me how to do this... I will spend some time today looking into it.
@RalphLipe I think your suggestion here would be sufficient:
Maybe just pass all the rule's constraints and let the called code sort stuff out.
We could set the API up as:
string Describe(Call call, PositionState, ps, List<Constraint> constraints)
Then use is
to check for the type of constraint we're looking for in ShowPoints.Describe
:
// if describing starting points, suppress the description if we also have a constraint for HCP
if (this._pointType == PointType.Starting) {
foreach (var constraint in constraints) {
if (constraint is ShowPoints as sp && sp._pointType == PointType.HighCard) {
return null;
}
}
}
This is due to an implementation detail to help the bidder understand "points" for later bids, but we really only need to communicate "HCP" in the descriptions (to avoid confusion/redundancy).
@RalphLipe I'd like to have us fix this particular issue before going live with descriptions, but since it's non-trivial I'd like your input on how you'd prefer to approach it. While we could suppress this in the code that aggregates the descriptions, that breaks the encapsulation you have. One alternative would be to pass the other constraints that are part of the rule to
Describe
so thePoints
constraint can suppress its own description for starting points if another constraint for HCP is present. It touches more code but is probably closer to the "right" fix. Thoughts?