PR: add alternative to fluent rule api that enables creating rules from data - 363

[dkent600]

This PR proposes a way to answer #363.

This is the second PR I've done on this. The first is here.

This PR is the same as the previous one, except that the conflicts with the master branch are resolved. (I am creating the new PR because (sheepishly) I deleted the repository on which the first PR was based.

Please refer to the first PR to see the original descriptions and discussion.

In response to @jdanyow's concern about changing the API, the only non-additive changes that I am aware of are that ValidationRules.ensure and ValidationRules.ensureObject are now each returning FluentRulesGenerator<TObject> instead of FluentEnsure<TObject>.

This should only affect someone who is compiling TypeScript and who explicitly expects FluentEnsure<TObject>. But I expect that the people most likely to be in that situation are the people mostly likely to welcome this code change, because it will simplify their code, likely absolving them of the need to care what types those two methods are returning (see the code example in the original PR).

Beyond that, the API is unchanged and works as before, only with more flexibility and fewer moving parts.

There is a small loss of contextual intellisense, but because the API is more flexible with this change, the intellisense is not so useful. There remain only four methods that must be called in a particular sequence:

• withMessageKey(key: string)
• withMessage(message: string)
• when(condition: (object: TObject) => boolean)
• tag(tag: string)

These four have to be called after at least one rule has been defined. This makes intuitive sense because they all act to modify a rule. I am throwing an exception if they are called too soon.

[dkent600]

@jdanyow

This code was originally intended to address #400, but #363 was naturally addressed by this change. Since I originally proposed this code, #400 got addressed separately.

You will note in the diffs that I have commented out the existing code that separately addresses #400. I don't believe that code is needed. Please let me know if you think there might be a use case I have missed.

[dkent600]

@jdanyow

The API change that is the subject of this PR, together with the API addition I have separately proposed to address #381, I believe yields an overall cleaner and more extensible API than what we have now.

See a summary of the #381 API addition below. Note, I added SatisfiesTaggedRuleParameters to address #279; it shows how this design is easily extensible, but could easily be done separately or not at all.

export type SatisfiesConditionArguments =
  SatisfiesConditionParameters | SatisfiesRuleParameters | SatisfiesTaggedRuleParameters;

/**
   * Applies a named or ad hoc rule to an ensured property or object.
   * @param SatisfiesConditionArguments Configuration for the rule. Can be either a SatisfiesConditionParameters, 
   * for ad hoc rules, or SatisfiesRuleParameters, for named rules, SatisfiesTaggedRuleParameters for tagged rules.
   */
  public satisfiesCondition(params: SatisfiesConditionArguments): FluentRulesGenerator<TObject>;

[jdanyow]

@dkent600 the fluent api was deliberately implemented so that methods become available at the appropriate times. This makes the API intuitive and discoverable for those using TypeScript. I think the change proposed here would be a step backwards for TypeScript users. For JavaScript users it probably wouldn't be as much of an impact although it's strange to have methods that throw in this way.

I really want to make an additive change that doesn't break compatibility with previous versions as described in #432 (comment)

[dkent600]

@jdanyow

Intuitive: The intuition is in how the concepts map to the API in a fluid and coherent way. Personally I don't see that this proposal loses any ground there -- I have never found the distinction between FluentRules and FluentRulesCustomizer to be clear nor useful in any intuitive way. But I understand that is a subjective thing.

Discoverable: All the methods are still discoverable so there is no loss there.

We lose the compile-time Typescript enforcement of certain rules -- enforcement that is simply no longer necessary and is in fact the source of the problem that we're trying to solve here.

As described above, the four rules where enforcement might be desired are now enforced immediately at run time (I don't see what is strange about that, we do it all them time when methods are called in an inappropriate context). Those four rules are quite intuitive in their use, so I don't see it as a big issue to lose their compile-time enforcement.

I note that this will be an improvement for JavaScript users who have neither compile-time nor run-time enforcement.

I don't see this small loss in compile-time enforcement as important as having a simpler and more flexible and extensible API.

Compatibility: As I mentioned, there is no compatibility loss that I expect will be noticed except (most likely) by those who will welcome this feature. But I understand if for some reason there is zero tolerance for breakage.

Having said all that, it is possible to restore the loss of compatibility and the original compile-time restrictions. What that means is that the only way to obtain the functionality required in this PR's use case would be to use ValidationRules.CreateFluentRulesGenerator() instead of the straight ValidationRules..

Would that be acceptable?

[jdanyow]

@dkent600 I think we're going to have to agree to disagree on whether we should expose methods in the fluent API when calling them would throw an RTE. I prefer the compile-time safety and intellisense, you would rather have a flat API and defer the checking until run-time.

I really want to make an additive change that doesn't break compatibility with previous versions as described in #432 (comment)

I think the most important part of this feature is coming up with a data structure that is flexible enough to model the standard rules and any custom rules developers might add.

[dkent600]

@jdanyow Did you see my proposal, at the end of my last comment, for a 100% additive solution that breaks nothing? Just like you asked for in #432 (comment) it is a single static method that is a wrapper around the existing API. However, it does not provide any new data structures -- I'm not clear why those are needed for this. My solution works fine without any new data structures.

If you don't feel like you understand what I'm proposing, take a look at the description on the original PR.

(As for new data structures, my proposal for #279 does provide new data structures, but they serve a different purpose, and I'm not sure whether they are the kind you're looking for. Would be happy to discuss. See #443 (comment))

Hmmm.... on further thought, maybe you're worried about the fact that the FluentRulesGenerator (and only it) doesn't use intellisense to enforce rules for the calling sequence with respect to withMessageKey, withMessage, when and tag? That is the only issue I can imagine. I've already described why I don't think that's important, and even less important since it would be limited to the use of FluentRulesGenerator, but yes perhaps we will have to agree to disagree on that.

If that is indeed the problem and you still want something different, can you please give an example of what it would look like?

[jdanyow]

ok- I'll reach out via Gitter later this week so we can sync up and I can get up to speed with the current plan

[dkent600]

Any news on this?

[dkent600]

I'm ready to discuss this PR anytime :-)

[EisenbergEffect]

Ping @jdanyow to follow up with @dkent600 Looks like this may have gotten buried.

[EisenbergEffect]

Bugging @jdanyow to follow up on this ;)

[jdanyow]

closing- never reached consensus on this