David,
I've been thinking about this more. Here are a couple of ideas I came up with:
1. Move the existing delegator cache maintenance methods to a separate interface+class. Add a method
to DelegatorInterface that retrieves an instance of the class for peripheral code to work with. End
result: +1 cache maint class, +1 delegator accessor method, -14 delegator cache maintenance methods.
2. Reduce all of the findByXxxx and findAll methods to a single find method that takes a
FindParameters class. A separate worker class could convert all of the various arguments currently
being sent to the myriad findXxxx methods and condense them into a FindParameters instance that is
passed to the single find method in the delegator. End result: +1 find parameters worker class, -23
delegator methods.
Both ideas simply move code out of the delegator into task-specific worker classes. I don't like the
idea of eliminating the various choices altogether, because that would cause even more code bloat in
the peripheral code.
-Adrian
David E Jones wrote:
This has been discussed before, but I thought it might be a good time
to bring it up again based on Adam Heath's recent additions to the
GenericDelegator (in revs r585808, r585803, r585802, r585759).
One of the issue with the Entity Engine that has been getting worse
over the years is method bloat. There are WAY too many variations of
different methods, IMO. These have been added for convenience, but the
net effect (that I don't think was expected or even considered) is that
when looking at the massive list it is tricky to figure out which
methods to use for what. This is a big problem for people new to the
Entity Engine, but also a problem for experienced EE users.
In short lately I'm thinking that having so many methods is worse than
the convenience they offer to make life easier for "lazy" coders.
Actually, with a decent IDE having lots of parameters isn't such a big
deal.
This does have the down side of requiring changes to lots of existing
code to use the fully featured methods instead of the simplified ones
(most of which just call the full featured ones with default values).
The up side is we end up with a really happy and clean GenericDelegator
class with only maybe 10-15 methods for different general operations,
and perhaps even less than that, like 5-10... (aside from the cache
maintenance methods, and other utility methods, many of which we might
want to make private, the general goal being to simplify the class).
In shorter I think this is getting to be one of those cases where less
is more...
Any thoughts? Should we start mass-marking methods as deprecated? Which
ones and in what forms of the methods should we leave?
-David