2013/3/6 Antonio Sanso <[email protected]>: > Hi Lukas > > On Mar 6, 2013, at 11:48 AM, Lukas Eder wrote: > >> 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. > > if I read correctly the code this method is called only at bundle activation > and binding
Yes, that's right - however briefly looking at the code, it smells as this cache is not really updated for an unbind. Regards Carsten > > Regards > > Antonio > >> 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 > -- Carsten Ziegeler [email protected]
