Is it "bad practice" to use event handlers for reuse?
Situation
This is a conceptual question and I'll try to be as clear as possible. Don't read a question if you have no interest in conceptual questions.
Ok, so there is some pre-written code that I am working on, and in this code, they reuse event handlers as some form of OOP.
An example of some kind of event handler:
protected void btnSomeButton_Click(object sender, Eventargs e)
{
//SOME LOGIC
}
An example of reusing the above event handler:
Note. This is called at an arbitrary point in the web application for the purpose of reusing // SOME LOGIC.
{
btnSomeButton_Click(null, null);
}
Some points for clarity
- The above method works.
- The event handler works in the normal sense.
I suppose this should be done
In my opinion, the correct way to do this would be to create a separate method and call that method when trying to reuse the code. (Feel free to correct me)
protected void btnSomeButton_Click(object sender, EventArgs e)
{
somefunction();
}
and code reuse:
{ somefunction() }
The method looks something like this:
void somefunction()
{
//SOME LOGIC
}
Question | My problem
I want to tell my boss why I want to change this code. All they can see is the code is working, "great", but looking at this coding style makes my eyes hurt.
What is the conceptual reason for the lack of coding in the coding method of the first example? Or is this acceptable and should I just leave it?
source to share
I agree with your suggestions to enter the code in a separate function.
The main problem I have with using repetitions like this is that it usually doesn't expect what looks like a handler for a framework event, which is called something other than a framework.
the parameters in your example indicate a very important point: they are not used right now, but this may change in the future. The structure usually gives certain guarantees for parameters, for example, that they will never be null
: when editing the code, you can say "why shouldn't I trust the structure?" and omit any checks null
. This will result in an error for the borderless code that just goes through null
.
Another problem that I often encounter is that handlers can be extended later with code that is unwanted or even harmful if called "manually". A simple example that comes to mind would record something like "button pressed by the user" when in fact the user did not.
I prefer to put every action that takes place in a frame handler in its own function, which has a meaningful name and can be called on its own.
source to share
Event handlers are loosely coupled, that is, the code that triggers an event doesn't know how to call it until the latter is registered.
If this method is then called from elsewhere in the code, passing it as Action<object, EventArgs>
and then calling it that way, the calling code still doesn't know about the method until it is entered, so it is still loosely coupled.
Calling an event handler directly, but tightly couples the event handler to the calling code. If you want to change the event handler later, you may end up breaking this code and you will have to check all references to it first (which might propagate throughout your code). Testing becomes more difficult as complex code is harder to separate into units, and therefore tests become more complex, require more ridicule and are much more fragile.
Event handlers should be private and should only be open to other parts of the code via "notify, don't ask" (for example, by registering it against the event).
source to share