1

I'm working on a .NET application and I'm wondering if I should use separate methods to handle the click events of two different buttons. They essentially do the same thing, just on different objects on the form, in this case two separate maps.

Same method: 

Private Sub SingleButtonHandler(sender As Object, e As EventArgs) Handles btnToggleSatNew.Click, btnToggleSatOld.Click
    If sender Is btnToggleSatNew Then
        Me.NewMap.ToggleSateliteView()
    Else
        Me.OldMap.ToggleSateliteView()
    End If
End Sub

Separate Methods: 

Private Sub btnToggleSatOld_Click(sender As Object, e As EventArgs) Handles btnToggleSatOld.Click
    Me.OldMap.ToggleSateliteView()
End Sub

Private Sub btnToggleSatNew_Click(sender As Object, e As EventArgs) Handles btnToggleSatNew.Click
    Me.NewMap.ToggleSateliteView()
End Sub

What's the overhead of checking if the two objects are the same? Is one version easier to read than the other? Or should I be doing something completely different?

Cuthbert
  • 201
  • 1
  • 2
  • 10

2 Answers2

7

Use separate methods. It makes the method easier to read, and makes it clearer what the method does. The SingleButtonHandler does two things based on a parameter: it toggles the new map if the sender is btnToggleStatNew, or toggles the old map. While it isn't a horribly messy method right now, it can grow into a large, unwieldy tangle of spaghetti code very quickly.

In addition, it opens up the question of "where should I stick this button's event?" With 1 click handler for two buttons, it sets a bad precedence. There will be the temptation to stick other button's click event into the one, because "this button's click is easy. It doesn't need another whole method" or "this button is kind of related to the others." Don't open up that temptation. Each button gets its own even handler.

On the performance overhead: Don't worry about it. Unless there's code that's causing major headaches, do not attempt to optimize. Premature optimization is the root of all evil. If the profiler says the code isn't the major time sink, then don't try to optimize it. Make it work. Make it readable.

Community
  • 1
CurtisHx
  • 1,177
2

Alternatively, you could add the map to be toggled as a property of the button, and then have one event handler which toggles the map given in the sender's property. This avoids an if/then statement, but gives you one method which does one thing.

HamHamJ
  • 497