Make the constructors of the Policy abstract base classes protected, not internal [allow custom policies]

[reisenberger]

The constructors of the Policy and Policy<TResult> abstract base classes are internal but should be made publicprotected.

This would allow users to implement new custom policies, outside of the Polly package, deriving from these base classes.

Needed in files Policy.cs, Policy.TResult.cs, PolicyAsync.cs, PolicyAsync.TResult.cs.

[rjongeneelen]

Please see pull request #299. Note that I have made more changes than this Issue suggested:

• The constructor access modifiers for Policy have been changed to protected (public is not needed)
• Some properties of Policy have been made public as they are commonly used to instantiate policies
• Almost all properties/methods of Policy have been made virtual, so they can be overridden in a derived Policy
• Some external properties (PolicyBuilder) and delegates have been made public as they are commonly used to instantiate policies

I've added comments for the newly-public constructors/methods/properties.

[udlose]

This at least provides the option to consumers of Polly to do so. However, I think it is an anti-pattern to extend an external API and expose its implementation throughout your code - even when doing so thru interfaces. Rather, it should be abstracted away and wrapped so the 3rd party API could be swapped at some future point.

…

On Aug 18, 2017 5:58 PM, "reisenberger" ***@***.***> wrote: The constructors of the Policy and Policy<TResult> abstract base classes are internal but should be made public. This would allow users to implement new custom policies, outside of the Polly package, deriving from these base classes. Needed in files Policy.cs, Policy.TResult.cs, PolicyAsync.cs, PolicyAsync.TResult.cs. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#298>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAoC9hRppZ0av9iaA8rHm0s0kiirxqnaks5sZhcigaJpZM4O8H9p> .

[rjongeneelen]

@udlose Agreed, but this is how it is currently setup.

Imho, I believe if you are architecting your own solution with the ability to swap the 3rd party API (Polly) out at any point in time, which is a great idea obviously, then you'd probably have to create an abstraction for the concept of 'policies' for your own application, e.g. a policy provider with a specific implementation that uses, and optionally implements additional, Polly policies.

My 2 cents.

[udlose]

@ExtRemo75 - yes, you will have to somehow abstract usages of policies. I was thinking of doing so via aggregation instead of inheritence. Though, I guess it's a matter of preference. Based on my experience, i've had more success using aggregation than inheritance to isolate 3rd party APIs.

[reisenberger]

Thanks all for all the great comments. Hoping to come back to this and #299 before the end of the week.

[reisenberger]

@udlose @ExtRemo75 Great discussion re aggregation rather than inheritance for isolating 3rd party APIs. I guess it depends what you are trying to achieve. If you want to isolate Polly, then yes, wrapping in your own abstractions is the way to go.

The benefit of allowing people to extend the Policy base class, is that they can implement just a little new behaviour, but also get thrown in for free all the scaffolding which Polly has - all the .Handle<>() syntax, all the .Execute() / .ExecuteAndCapture() overloads, all the events/metrics stuff as we get to add it, etc. But yes, extending that way implies a certain investment in sticking with Polly.

The fact that all this functionality is loaded on the Policy base class is something of a consequence of how the architecture has evolved (including before my time). In a hypothetical complete rewrite of Polly, it'd be possible to component-ise all that more - IFaultFilters, IPolicyBehaviour, all brought together with an IExecutionDispatcher or similar (I have versions of this in my head). This could avoid some of the compromises like extension by extending the base class. Such a complete rewrite would however represent a significant time commitment.

We would be very happy to see anyone's ideas though - we can also make incremental change. The current approach is targeting incremental change - some of it quite significant (eg #281) - where the existing architecture is actively holding us back.

Keep the thoughts coming, as we have opportunities to change the architecture with v6.

[reisenberger]

Closed by #391

The options for compenentisation discussed in the previous comment (thanks for the valuable discussion) remain options for a future version of Polly or any similar product. As discussed above, a rewrite to such a structure would be intellectually well motivated (and does appeal). It would require a significant chunk of time tho (and would lead to a significantly different syntax experience for users).

( EDIT: Very happy to discuss any ideas around this, and/or support anyone who wants to explore them. )