delphi - Why is it bad practice to call an eventhandler from code?

Question

Say you have a menu item and a button that do the same task. Why is it bad practice to put the code for the task into one control's action event and then make a call to that event from the other control? Delphi allows this as does vb6 but realbasic doesn't and says you should put the code into a method that is then called by both the menu and the button

Answer 1

It's a question of how your program is organized. In the scenario you've described, the menu item's behavior will be defined in terms of the button's:

procedure TJbForm.MenuItem1Click(Sender: TObject);
begin
  // Three different ways to write this, with subtly different
  // ways to interpret it:

  Button1Click(Sender);
  // 1. "Call some other function. The name suggests it's the
  //    function that also handles button clicks."

  Button1.OnClick(Sender);
  // 2. "Call whatever method we call when the button gets clicked."
  //    (And hope the property isn't nil!)

  Button1.Click;
  // 3. "Pretend the button was clicked."
end;

Any of those three implementations will work, but why should the menu item be so dependent on the button? What's so special about the button that it should define the menu item? If a new UI design did away with buttons, what would happen to the menu? A better way is to factor out the event handler's actions so it's independent of the controls it's attached to. There are a few ways to do that:

1. One is to get rid of the MenuItem1Click method altogether and assign the Button1Click method to the MenuItem1.OnClick event property. It's confusing to have methods named for buttons assigned to menu items' events, so you'll want to rename the event handler, but that's OK, because unlike VB, Delphi's method names do not define what events they handle. You can assign any method to any event handler as long as the signatures match. Both components' OnClick events are of type TNotifyEvent, so they can share a single implementation. Name methods for what they do, not what they belong to.

2. Another way is to move the button's event-handler code into a separate method, and then call that method from both components' event handlers:

   procedure HandleClick;
   begin
     // Do something.
   end;
   
   procedure TJbForm.Button1Click(Sender: TObject);
   begin
     HandleClick;
   end;
   
   procedure TJbForm.MenuItem1Click(Sender: TObject);
   begin
     HandleClick;
   end;
   

   This way, the code that really does stuff isn't tied directly to either component, and that gives you the freedom to change those controls more easily, such as by renaming them, or replacing them with different controls. Separating the code from the component leads us to the third way:

3. The TAction component, introduced in Delphi 4, is designed especially for the situation you've described, where there are multiple UI paths to the same command. (Other languages and development environments provide similar concepts; it's not unique to Delphi.) Put your event-handling code in the TAction's OnExecute event handler, and then assign that action to the Action property of both the button and the menu item.

   procedure TJbForm.Action1Click(Sender: TObject);
   begin
     // Do something
     // (Depending on how closely this event's behavior is tied to
     // manipulating the rest of the UI controls, it might make
     // sense to keep the HandleClick function I mentioned above.)
   end;
   

   Want to add another UI element that acts like the button? No problem. Add it, set its Action property, and you're finished. No need to write more code to make the new control look and act like the old one. You've already written that code once.

   TAction goes beyond just event handlers. It lets you ensure that your UI controls have uniform property settings, including captions, hints, visibility, enabledness, and icons. When a command isn't valid at the time, set the action's Enabled property accordingly, and any linked controls will automatically get disabled. No need to worry about a command being disabled through the tool bar, but still enabled through the menu, for example. You can even use the action's OnUpdate event so that the action can update itself based on current conditions, instead of you needing to know whenever something happens that might require you to set the Enabled property right away.