patternjavaMinor
Replicating C#-style events in Java using reflection
Viewed 0 times
reflectioneventsreplicatingjavastyleusing
Problem
For something I'm doing, it would just make things so much easier, quicker, and cleaner to have C# style events. I've written a class I believe should replicate a C# style event using reflection. Since I've never actually used reflection before in any meaningful sense, I'm very much not happy using it without, at the very least, a second opinion.
I want to declare an event with:
Raise the event (presumably within the declaring class) with (where SomeArguments is whatever class that may be specified in the Javadocs boxed in an Object):
Add a method to be called when the event is raised with:
Stop a method being called when an event is raised with:
And cancel all listeners with:
Methods to listen to the event should be formatted like:
And this is my class:
```
package puppy.hanii.library;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
public class Event
{
public Event()
{
Methods = new ArrayList();
DeclaringClasses = new ArrayList();
}
ArrayList Methods;
ArrayList DeclaringClasses;
public boolean AddListener(Object Class, String MethodName)
{
try
{
Method MTR = Class.getClass().getMethod(MethodName, Object.class, Object.class); // MTR = Method To Register.
if(Methods.contains(MTR))
return false;
else
{
Methods.add(MTR);
DeclaringClasses.add(Class);
return true;
}
}
catch (NoSuchMethodException error) { throw new RuntimeException("No such method, or correct overloads of method, exists.")
I want to declare an event with:
public Event SomeEvent = new Event();Raise the event (presumably within the declaring class) with (where SomeArguments is whatever class that may be specified in the Javadocs boxed in an Object):
SomeEvent.Raise(this, SomeArguments);Add a method to be called when the event is raised with:
SomeEvent.AddListener(this, "onSomeEvent");Stop a method being called when an event is raised with:
SomeEvent.RemoveListener(this, "onSomeEvent");And cancel all listeners with:
SomeEvent.ClearListeners();Methods to listen to the event should be formatted like:
public void onSomeEvent(object Sender, object SomeArguments)
{ }And this is my class:
```
package puppy.hanii.library;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
public class Event
{
public Event()
{
Methods = new ArrayList();
DeclaringClasses = new ArrayList();
}
ArrayList Methods;
ArrayList DeclaringClasses;
public boolean AddListener(Object Class, String MethodName)
{
try
{
Method MTR = Class.getClass().getMethod(MethodName, Object.class, Object.class); // MTR = Method To Register.
if(Methods.contains(MTR))
return false;
else
{
Methods.add(MTR);
DeclaringClasses.add(Class);
return true;
}
}
catch (NoSuchMethodException error) { throw new RuntimeException("No such method, or correct overloads of method, exists.")
Solution
ArrayList Methods;
ArrayList DeclaringClasses;Two things: firstly, you're using C# naming conventions rather than Java. Secondly, code to the interface: the fields should be
List rather than ArrayList.public boolean AddListener(Object Class, String MethodName)Class isn't an obvious name for something which isn't a class. And given that you're using java.lang.Class inside the method without fully qualifying the name, it has even more potential to confuse.Method MTR = Class.getClass().getMethod(MethodName, Object.class, Object.class); // MTR = Method To Register.If you have to add a comment explaining the name, perhaps you chose the wrong name? What's wrong with
Method method = ...?if(Methods.contains(MTR))
return false;So I can't have the event notify two different objects of the same class?
public boolean RemoveListener(Object Class, String MethodName)
{
try
{
DeclaringClasses.remove(Class);
return Methods.remove(Class.getClass().getMethod(MethodName, Object.class, Object.class));
}Buggy.
Collection.remove removes one instance of the object, but you're making no attempt to ensure it's the right one. If I register two methods on the same object, this could remove the wrong instance of the object, which would then make objects and methods not line up correctly.In fact, it's even worse than that: if I register a method on one object and then remove it on another, or register one method on an object and then remove a different one, I'll end up with
Methods and DeclaringClasses not even being the same size.Also, why would you want to call
getMethod again unless the method hasn't been registered? If the method has been registered, you've already resolved it; if it hasn't, you don't need to resolve it except to check for typos.The best way to store the resolved methods is probably
Map, Method>: that allows you to iterate through the entry set in the Raise method and to efficiently implement RemoveListener.catch(IllegalAccessException error)
{ }No. Swallowing exceptions like that is bad, especially when you've done nothing to prevent them occurring. You should check in
AddListener whether the method is accessible, and throw an exception there if it isn't. Then here you should log an error for debugging purposes.Other issues to consider:
-
Thread-safety. This has no thread-safety whatsoever.
-
Typing. If you're going to emulate C#'s events then don't just emulate the ungenericised
EventHandler. At the very least, emulate EventHandler. Because Java's generics aren't reified this requires a bit of boilerplate:public class Event
{
private final Class typeParam;
public Event(Class typeParam)
{
this.typeParam = typeParam;
}You can now use
typeParam in the Class.getMethod call.Code Snippets
ArrayList<Method> Methods;
ArrayList<Object> DeclaringClasses;public boolean AddListener(Object Class, String MethodName)Method MTR = Class.getClass().getMethod(MethodName, Object.class, Object.class); // MTR = Method To Register.if(Methods.contains(MTR))
return false;public boolean RemoveListener(Object Class, String MethodName)
{
try
{
DeclaringClasses.remove(Class);
return Methods.remove(Class.getClass().getMethod(MethodName, Object.class, Object.class));
}Context
StackExchange Code Review Q#15549, answer score: 3
Revisions (0)
No revisions yet.