Could json logic operators be protected interal?

[dcook-net]

Hi,

I was wondering if there was a technical reason for marking the Json Logic operators as internal?

We are trying to extend the library a little for internal use, and would like to override the behaviour of several comparison operators in certain circumstances, so that if the data being operated on meets a certain condition, ie, it looks like a Date, then we perform our own comparison, otherwise, we defer to the code that already exists in say <, >, =>, =< for example.

Currently, we can't inherit from or instansiate these operators, without maybe resorting to reflection.

Would you be open to the idea of changing the access modifiers on operators, and thier fields to protected internal?

I can raise a PR for this work if it's something that works for you.
If this is not desirable for you, can you suggest an alternative?

For reference, here is a sample of what we are trying to achieve;

[Operator(">")]
public class CustomMoreThanRule : Json.Logic.Rules.MoreThanRule
{
     public CustomMoreThanRule(Rule a, Rule b) : base(a, b)
     {
     }

     public override JsonNode? Apply(JsonNode? data, JsonNode? contextData = null)
     {
         var a = A.Apply(data, contextData);
         var b = B.Apply(data, contextData);

         // ignoring for now what happens when only one is a date!
         if (a.IsDateValue() && b.IsDateValue())
             return CompareDates(a, b);

         return base.Apply(data, contextData);
    }
}

Thanks,
Dave

[gregsdennis]

I was wondering if there was a technical reason for marking the Json Logic operators as internal?

It's more a philisophical reason. The primary mechanism for creating custom behavior is through defining custom rules. If you register an operation that already exists, it'll overwrite with your rule.

However, I can see an argument for wanting to actually override the rule class in order to take advantage of the default behavior except for certain cases.

I was going to suggest a composition approach for now, but that requires that you be able to access the MoreThanRule constructor, which is also internal.

Yeah, I think internal protected is a good option. Give me a couple days, and I'll have an update out.

[dcook-net]

I agree that composition approach would be preferable, though that would require making the classes and their constructors public.
You could also enforce composition over inheritance by sealing the operator classes.

[dcook-net]

Created a PR for the internal protected approach.
Happy to apply changes if this isn't what you were thinking.

Cheers,
Dave

[gregsdennis]

Thanks for the PR. I'll have a look over it today.

I agree that composition approach would be preferable

My philosophical objection wasn't so much one of "composition over inheritance" as these operations are well-defined. Making an operation do something else isn't interoperable. However, you do provide a good use case, and so long as you understand the interoperability implications of your change, I shouldn't prevent you from making it.

[dcook-net]

Ah, I see whay you mean. Yes, I agree.

I've added to the docs to explain those implications.