Hello,

I've been stepping through the SlingPostServlet, having a look at its
code. I feel that there is a potential "code smell" in some of its
methods. As I'm unsure how the Sling project deals with such
situations, I felt that posting a mail here before creating an issue
might be better.

The code I'm talking about is contained in
registerPostResponseCreator(), where the cachedPostResponseCreators
attribute is initialised. There are two issues here:

1. The cachedPostResponseCreators array is re-initialised every time
this method is invoked. This leads to an unnecessary O(n^2)
initialisation complexity on componentContext.locateService() method.
I guess that this cache's whole purpose is to avoid expensive
locateService() calls, so this should probably be fixed.

2. A first loop initialises this cachedPostResponseCreators array. In
a second step, "null" entries are "removed" by copying the array. This
behaviour is hard to read from the existing code, which is a bit
verbose. In order to avoid the potential for bugs, I'd like to suggest
using the (untested) behaviour from the attached patch. Note that
other methods, such as registerPostProcessor(),
registerNodeNameGenerator() suffers from similar "verbosity problems".

Cheers
Lukas

Reply via email to