Reflection_Engine: allow for disabling recording the events

[pawelbaran]

Description:

Together with @tiagogrossi we just approached an interesting problem (actually, I have been there before myself, but never picked it up so far). Namely, we have the following pseudocode trying to offset PolyCurve loop, and if the offset is self-intersecting then returning its bounding rectangle instead:

PolyCurve offset = loop.Offset(1);
if (offset.IsSelfIntersecting())
    offset = //create rectangle;

In case of resultant self-intersection, Offset method starts recording warnings, which is valid. However, in our case, we do not want to expose them to the user because the offset is not a part of the final output (it is overwritten by the rectangle).

So ideally, we would like to be able to instruct Reflection not to record events for a moment:

BH.Engine.Reflection.Compute.StopRecordingEvents(EventType.Warning);
PolyCurve offset = loop.Offset(1);
if (offset.IsSelfIntersecting())
    offset = //create rectangle;

BH.Engine.Reflection.Compute.StartRecordingEvents(EventType.Warning);

What do you think about it? Technically this could be realised using a private bool field checked on calls to RecordEvent and switched using StopRecordingEvents and StartRecordingEvents. This is however far from perfect, firstly because of the private field being shared by a few methods and secondly likely to become problematic in case of multithreading.

Happy to hear others' opinion @al-fisher @adecler @FraserGreenroyd @alelom @IsakNaslundBh

[al-fisher]

@alelom think this idea is not dissimilar from ideas you have had in the past for suppressing/controlling warnings when issues from an internal method call are caught by a wrapping method. No?

[alelom]

Glad that this issue has popped up for someone other than me, as it adds the necessary pressure to solve it 😄

I have been vocal about this limitation of the BHoM framework in the past. Our current Reflection.Record* approach effectively mixes together three things: user alerts, error logging and exception handling. This works perfectly as long as methods are used independently from each other and directly from an User – i.e. called from the UI as individual "components". However, when method needs to be called from another method – in this case Offset() – this kind of issues with how we deal with errors and their exposure is bound to happen in certain situations.

@pawelbaran correctly identifies a quick & dirty solution to the problem – the developer to manually mute the callee warnings. This was also one of the alternatives proposed during the Error Management workshop we had some time ago when I raised the issue (notes here). I'd be happy to implement this, although I'd prefer we pushed a solution ​that offers the full functionality of native C# catch: muting the events means losing the event. What if you wanted to aggregate a set of warnings into a single one?

We had created a set of issues around this. I propose to prioritize them and devise proper solution to this problem. I will create a BHoM_admin collection issue.

In the short term, we could add a muting as @pawelbaran proposes, but on the long term I think we need to "reinvent" C#'s catch to an extent, in order to avoid imposing limitations on development.

[adecler]

I agree with @alelom , I would prefer we directly implement a solution with better functionality. Otherwise, we might end up having to do another set of PR coordination just to replace the temporary solution.

What about using a bookmark system instead ? So the methods we would use would look something like this:

• EventBookmark GetEventBookmark()
• bool ReturnToEventBookmark(EventBookmark bookmark)
• List<Event> NewEvents(EventBookmark bookmark, EventFilter filter = null)

The bookmark could obviously be a simple integer (index in the event list) but I would prefer it to also contain something like the BHoM_Guid of the last event for added safety (if the event list get cleared by another method for example).

I have added the NewEvents method so we have similar capability to the C# catch system as well. Remember that we already have a few sub-classes of Event (e.g. VersioningEvent) so the filter could be both on event level (warning, error,...) and event class.

Finally, it is worth considering whether we want to provide both solutions (bookmark and StopRecordingEvents) or not. I like that StopRecordingEvents allows for a concise way to prevent recording of warning without blocking recordings of errors. The bookmark system can achieve the same thing (and with more control) but is a bit more verbose (similarly to a try-catch). So there might be value to have both.