Gui.cs: Button.Clicked is Action and not an event?

Created on 7 Jun 2018  路  17Comments  路  Source: migueldeicaza/gui.cs

Why is Button.Clicked (and I assume other events) implemented as Action but not as an event?
I'd say event is more natural and standard.

design

Most helpful comment

Good point folks - I am on vacation this week, with limited access to the internet, but will join this conversation next week. There seem to be two issues, one of consistency (public delegate vs event) and whether the signature should be EventHandler or the newer Action/Func pattern.

Will ponder here next week.

All 17 comments

Same is true of RadioGroup.SelectionChanged but not TextField.Changed

Should I create a PR to fix this by converting Action to EventHandler?

I have just never appreciated EventHandler types. They were invented in an era that lacked lambdas. My problem with it is two fold: the first parameter is rarely used and when you need it lambdas are now available. The second is that the payload to the event rather than being just a value is another type, this is just too much boilerplate in my view.

Am I missing something?

The benefits of events as I see it are:

  1. The familiarity of the pattern to people who use the .NET Framework libraries that implement events
  2. The more obvious/less cumbersome way to attach and detatch multiple handlers
  3. The ability to control how you store multiple handlers (by overriding the event's add and remove handlers)
  4. The built in pattern of communicating things back to the caller or between different handlers

I'd say the latter three are pretty niche cases, though, at least in the context of a UI library. Personally, I don't have strong feelings either way, other than a bias for consistentcy

I share Miguel's scorn for events.

That said, consistency is a good thing. Sometimes the right answer is to make things consistently awful. If so, events are the perfect way to achieve that.

I feel they are as noise.

var btn = new Button();
btn.Click += () => Console.WriteLine("Clicked");

//VS
var btn = new Button();
btn.Click += (sender,args) => Console.WriteLine("Clicked");

//Long
var btn = new Button();
btn.Click += Clicked;

void Clicked() => Console.WriteLine("Clicked");

//VS
var btn = new Button();
btn.Click += Clicked;

void Clicked(object sender, EventArgs args) => Console.WriteLine("Clicked");

Sorry, I meant consistency within the library (TextField.Changed is event vs RadioGroup.SelectionChanged is Action). I agree that events don't bring much to the table here.

So if for consistency, then pls go for action, there is almost no need for the sender args pattern. :)

There are other reasons for events instead of actions besides ones mentioned by @pmsanford.
On top of my head:
Such as converting events to Tasks or using them with Reactive stuff - I think there are utilities that handle these easily with events. Not sure about actions, but it shouldn't be hard anyway.
Also, what if I want to handle multiple buttons with a single method? I'd cast sender to button and then do whatever I'd do.

You can do that yourself without the as/null check or is with a closure like

void HandleClicked(Button button) => Console.WriteLine($"Clicked {button.Id}");

...

var btn = new Button() { Id = "HappyButton" };
btn.Click = () => HandleClicked(btn);

Sure, it's doable. But is it optimal? Compiler creates a hidden class just for that.

Good point folks - I am on vacation this week, with limited access to the internet, but will join this conversation next week. There seem to be two issues, one of consistency (public delegate vs event) and whether the signature should be EventHandler or the newer Action/Func pattern.

Will ponder here next week.

Introducing events would be a lie. We do not have concurrent tasks, no UI thread in the background, no devices sending something. When it comes to the terminal, we have an endless loop, polling the keyboard and the mouse - which means essentially waiting for the user - and meanwhile doing things.

Regarding the interaction between menu and a statusbar, I'd really appreciate to use new StatusBarMessage(menuItem.HelpText) to send sth to the bar. But it doesn't feel right here.

Not sure if that somehow interferes with the reactive stuff...

FWIW, I vote for events. They're standard, a lot of libraries and things are built on top of them. And I'm not sure it's possible to remove an action. Whereas with events I can remove a handler via -=.

You can remove an action by clearing it: control.Action = null

But I hear you, you want events, I need to digest this, I think I will make the change, but it will break people

Sorry I didn't see this Issue when I created this one: https://github.com/migueldeicaza/gui.cs/issues/447

I'm clearly in the EventHandler camp. For all the reasons above, plus: Action doesn't support multiple subscribers. EventHandler is designed to be used in a pub/sub way.

I think Action is awesome when used for internal-details. But for public APIs (and Terminal.Gui is a public API) EventHandler should be the norm.

Dupe of #447

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tig picture tig  路  6Comments

abiratur picture abiratur  路  7Comments

0x788 picture 0x788  路  3Comments

TylerLeonhardt picture TylerLeonhardt  路  5Comments

alet picture alet  路  6Comments