1

My WPF Custom Control handles dragging shapes across an image on a canvas. Some shapes may be dragged beyond the limits of the image but others should be limited. But the control itself does not know the rules, only clients (i.e. the XAML page that created the control or its View-model) knows what they are.

So I need to give this control a way to let clients know that shapes are about to be dragged so that clients may limit drag for the duration of the drag operation

I had the idea for a RoutedEvent (e.g. ItemDragStarting) that my control could raise when shape dragging begins. I reasoned that we already know that event handlers traditionally set the RoutedEventArgs.Handled property to stop further processing of the event. That seems basically like an "OUT" parameter doesn't it?

So maybe my RoutedEventArgs object of my ItemDragStarting event could expose a "ValidLimitRect" parameter to allow clients to optionally set the valid limits. After the event fired, the control could check this and, if set, limit the drag for this one drag operation.

For example:

// User has started dragging items.  Raise the event with no limit rect

var args = new DragStartingEventArgs(itemsBeingDragged, Rect.Empty);
RaiseEvent(args)

// Did any of the clients set the limit rect to anything? 

if (!args.LimitRect.IsEmpty)
{
     // Save the value to limit dragging 
}

But I've never before made event-firing code actually care about the end state of its arguments. Until now they've always been fire-and-forget. So I am wondering if this is even a good idea.

Is this anything resembling standard practice in WPF or is this code-smell? If it is the latter, what's the better way to do this?

Guru Stron
  • 102,774
  • 10
  • 95
  • 132
Joe
  • 5,394
  • 3
  • 23
  • 54
  • If I understand correctly, you want to go from the control to the client. Let the client hydrate the state and then take that back to the control. It seems like a job for two events, not one. – Funk Sep 14 '22 at 06:20
  • Have a look at [WPF input events](https://learn.microsoft.com/en-us/dotnet/desktop/wpf/events/routed-events-overview#wpf-input-events): "This handling behavior is useful for composite controls designers who want hit-test based input events or focus-based input events to be reported at the top-level of their control. Top-level elements of the control have the opportunity to class-handle preview events from control subcomponents in order to "replace" them with a top-level control-specific event.". – Funk Sep 14 '22 at 06:20
  • 1
    This is a valid pattern. In addition, many .NET classes use feedback events aka pre-events. For example, Window.Closing and Window.Closed make a pair of events where the first feedback event allows the observer to cancel the following event. – BionicCode Sep 14 '22 at 06:22
  • Since events should never return a value (they must be void), observing the event argument is the only chance to interact with the observers in the course of an event. – BionicCode Sep 14 '22 at 06:24
  • Note that when the argument is a shared instance, the same instance is modified by all observers. Otherwise you would have to track every instance of the event argument e.g., by storing them in a collection. Alternatively, access the invocation list to invoke each handler explicitly to get more control. You would then check the argument before the next handler invocation (which is what .NET is doing in case of routed events). Events like the Window.Closing event use a shared event argument instance, for example. – BionicCode Sep 14 '22 at 06:37
  • @BionicCode A tunneling event followed by a bubbling event is indeed a valid pattern. I'm questioning the use of the pattern to have a top-level element change the state of the args of a tunneling event, to have that state transferred back to the sender. As mentioned in the quote from my previous comment, that top-level element should start it's own event to do that. – Funk Sep 14 '22 at 07:03
  • @Funk I think there are some good scenarios where it could make sense for the event source to know if the event was handled. And some times it even makes sense to give the handlers the chance to return a value. In my opinion it really depends on the scenario. Sometimes, maybe most of the times, it makes more sense to let the handler invoke a method on the event source to respond to the event. In case of the common pre-event pattern, where the event source can't leave the current context to allow the handlers to call an e.g. AbortCancel() method, returning a response value makes perfect sense. – BionicCode Sep 14 '22 at 07:19
  • @Funk In this special scenario of the question, the author must know if he wants to allow random handlers to change the state of the event source. In fact, the response is not a simple flag. The response in the given example introduces side effects. Given that the event is raised in context of an invoked operation, allowing to change the state by other instances than the caller could lead to unwanted side effects. I agree that configuring an object using events, like in the example in the question, does not provide a robust design. But in general, this is a valid pattern and not a code smell. – BionicCode Sep 14 '22 at 07:20
  • @Funk The quote you mentioned means that you should avoid marking routed events as handled as this has a severe impact on the behavior of the application. It's usually not the responsibility of an event handler to decide for other handlers if they are allowed to handle the event too. It should be an exception, like cancelling the event to raise a more specific event to replace the original event. It's meant as a design guide and makes perfect sense. For example this allows controls to hide events to replace them with more specific events. – BionicCode Sep 14 '22 at 07:25
  • @Joe While the pre-event pattern is a very common and useful pattern in general, I have to agree with Funk that your given example doesn't depict the best scenario to implement an event that expects a feedback argument. This is because the event allows the handlers to change the state of the event source. This means, the result of the ongoing operation is not deterministic from the original caller's point of view. The produced result or the object's state is now random. – BionicCode Sep 14 '22 at 07:44
  • This is because the operation that actually raises the event and which was started by the original caller can now transition the object into an unexpected state producing unexpected results. You must think very carefully if you want to allow this. The common way would be to let the caller define the state e.g., by setting properties. The invoking an operation produces predictable results. In other word, events should not be used to configure an object. – BionicCode Sep 14 '22 at 07:44
  • Imagine a calculator where you enter 2 * 2 and the output is 32000, because the calculator allows to modify the parameters silently by observers of the original operation. It's not deterministic. You can't trust such a calculator. But in general, observing the event argument to allow the handlers to set a e.g. flag is not a code smell. But always be careful. Eventhandlers (event delegates) are `void` in the first place for a very good reason. – BionicCode Sep 14 '22 at 07:54
  • @BionicCode I'm not sure that is what is meant by the quote, but you did cover all the important bases in your comments. Hydrating the shared args, along the way back to the source, doesn't feel like a very robust design. – Funk Sep 14 '22 at 07:58
  • Wow. Lots of activity here. I appreciate all the thought and time you all took to discuss this. I am pretty certain now that this approach would be a bad idea. I think it might "work" but again, there's that code smell. However, since the control is a WPF `MultiSelector` (and therefore also a `Selector`) another approach I could use would be to give it a new `DragLimitRect` DependencyProperty that the client could set at will, perhaps in response to the existing`SelectionChanged` events that all `Selector`s have. That seems a whole lot more robust – Joe Sep 14 '22 at 15:42

0 Answers0