hi,
I was playing with the quartz jobs in the backend and I thought this
is an area where some simplification and/or cleanup would be useful.
- SchedulerUtil interface would be nice to hide quartz from the rest
of the code, but it very rarely used, the clients are bound to it's
single implementation, SchedulerUtilQuartzImpl through it's
getInstance() method.
I think the whole class name is misleading, since usually when I imagine a utils class,
it's a simple class that does some menial work for me in static methods, and not
really calls anything else or even has an instance.
Maybe the class can be renamed to just Scheduler, or ScheduleManager which will be more
precise.
- It was designed to be a local EJB, Backend actually expects it to
be injected. (this field is not used)
- when scheduling a job, you call schedule...Job(Object instance,
String methodName, ...) however, it is not the _methodname_ that
the executor will look for
- instead, it will check the OnTimerMethodAnnotation on all the
methods. But this annotation has everywhere the methodName as value
- JobWrapper actually iterates over all the methods to find the one
with the right annotation
So a quick simplification could be:
- The annotation is not needed, it could be removed
- JobWrapper could just getMethod(methodName, argClasses) instead of
looking for the annotation in all of the methods
Sounds good, or maybe just keep the annotation and not the method name in the
call/annotation since then if the method name changes it won't break and we can easily
locate all jobs by searching for the annotation..
- I am really not for factoryes, but if we want to separate the
interface from the implementation, then probably a
SchedulerUtilFactory could help here. The dummy implementation
would do just the very same thing as the
SchedulerUtilQuartzImpl.getInstance()
- I would remove the reference to SchedulerUtil from Backend as
well, since it is not used. Really _should_ the Backend class do
any scheduling?
Backend does schedule at least one job in it's Initialize() method..
Maybe the class should be injected, but I don't know if that happens so maybe
that's why it's unused.
Please share your thoughts.
Thank you,
Laszlo
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel