FYI: Takuya and I figured out a solution that was better than the Junit approach that was previously proposed.
We both think that a server startup hook would be a useful feature for Manager type classes. However, I am still not sure if it is really needed.
thanks, aaron
At 04:49 PM 4/21/2005, you wrote:
> Ok, this is what I understand; the ReviewIdGroupManager.getInstance() isn't > executed until someone goes to Review ID Group Management Page. Which > sounds fine to me. Why is that a problem? Is that problem because your > analyses needs to use the ReviewIdGroupManager too? If so, then why doesn't > the analyses have to invoke the ReviewIdGroupManager.getInstance() ? Then > that would ensure that the ReviewIdGroupManager is correctly initialized > before anyone needs to use it.
This is problem because only review-group-assigned Review Id is shown in the Review ID Group management page.
Currently it's problem in the following situation:
1) A user goes to the Review Group Management page, and delete a existing ReviewGroup as soon as hackystat starts up. Since ReviewIdGroup class which implements ReviewGroupListener is not added to the ReviewGroup's, then the updated status (the existing review group deletion) was not reflected to the ReviewIdGroup.
> Your code should look like; > > /** Instantiates the singleton instance. */ > private ReviewGroupManager() { > this.reviewGroupMap = read(); > this.listeners = new ArrayList(); > } > > private ReviewIdGroupManager() { > // instantiates this to make sure ReviewGroup instances are ready to > be accessed. > ReviewGroupManager.getInstance().addListener(this); > ReviewIdManager.getInstance().addListener(this); > this.reviewIdGroupMap = read(); > initializeReviewIdGroupMap(); > } >
This is alreay in the CVS. The code still would be problem without server startup mechanism or without invoking the test case.
> I can't delve into your code immediately, but from what I see below, it seems like a > redesign of your code might be the best way to solve your situation. If an informal > discussion with Aaron isn't enough, let me know and I'll try to schedule some time to > look at it.
Currently, Philip is invoking the test cases after the new public release so that it would be no problem. I might need Philip's help too to solve this if we need to avold the problem above.
On Thu, 21 Apr 2005 13:43:16 -1000 Aaron Kagawa <[EMAIL PROTECTED]> wrote:
> > >However, I am trying to instatiate the class in a testcase so tha it > >would be be fine for the UH hackysatt public server if junitAll is > >always invoked after the hackysat is deployed. > > This is probably not a good solution. > > > >The problem is that the ReviewIdGroup.getInstance() (which sets several > >listeners in the constructor) is not called (loaded) until someone take > >a look at the review ID Group management page, which is not in the > >hackystat top page (e.g. preference page), rather exists from the next > >page I created, and is forwarded from the manage action from the top > >page. Because of this, the ReviewIdGroup class is not loaded until > >someone use the class even I use the signelton in the class. > > Ok, this is what I understand; the ReviewIdGroupManager.getInstance() isn't > executed until someone goes to Review ID Group Management Page. Which > sounds fine to me. Why is that a problem? Is that problem because your > analyses needs to use the ReviewIdGroupManager too? If so, then why doesn't > the analyses have to invoke the ReviewIdGroupManager.getInstance() ? Then > that would ensure that the ReviewIdGroupManager is correctly initialized > before anyone needs to use it. > > I looked at your code that is currently in CVS. > > /** Instantiates the singleton instance. */ > private ReviewGroupManager() { > this.reviewGroupMap = read(); > this.listeners = new ArrayList(); > this.addListner(ReviewIdGroupManager.getInstance()); > } > > private ReviewIdGroupManager() { > // instantiates this to make sure ReviewGroup instances are ready to > be accessed. > ReviewGroupManager.getInstance(); > ReviewIdManager.getInstance(); > this.reviewIdGroupMap = read(); > initializeReviewIdGroupMap(); > } > > So, ReviewGroupManager depends on ReviewIdGroupManager AND > ReviewIdGroupManager depends on ReviewGroupManager? That seems strange to > me. I thought we fixed this yesterday? > > Your code should look like; > > /** Instantiates the singleton instance. */ > private ReviewGroupManager() { > this.reviewGroupMap = read(); > this.listeners = new ArrayList(); > } > > private ReviewIdGroupManager() { > // instantiates this to make sure ReviewGroup instances are ready to > be accessed. > ReviewGroupManager.getInstance().addListener(this); > ReviewIdManager.getInstance().addListener(this); > this.reviewIdGroupMap = read(); > initializeReviewIdGroupMap(); > } > > thanks, aaron > > At 11:54 AM 4/21/2005, you wrote: > >Hi Philip and Aaron, > > > >The problem is that the ReviewIdGroup.getInstance() (which sets several > >listeners in the constructor) is not called (loaded) until someone take > >a look at the review ID Group management page, which is not in the > >hackystat top page (e.g. preference page), rather exists from the next > >page I created, and is forwarded from the manage action from the top > >page. Because of this, the ReviewIdGroup class is not loaded until > >someone use the class even I use the signelton in the class. > > > >private static ReviewIdGroup theInstance = new ReviewIdGroup(); > > > >However, I am trying to instatiate the class in a testcase so tha it > >would be be fine for the UH hackysatt public server if junitAll is > >always invoked after the hackysat is deployed. > > > >Cheers, > > > >takuya > > > >On Thu, 21 Apr 2005 06:32:26 -1000 > >Philip Johnson <[EMAIL PROTECTED]> wrote: > > > > > Takuya, > > > > > > I kind of agree with Aaron. While I don't have an issue with implementing > > > a startup hook mechanism, why can't you use the normal lazy instantiation > > > mechanism: > > > > > > ReviewIdGroup.getInstance() > > > > > > which on it's first invocation does all of the instantiation you want? > > > > > > Cheers, > > > Philip > > > > > > --On Wednesday, April 20, 2005 10:51 PM -1000 Aaron Kagawa > > > <[EMAIL PROTECTED]> wrote: > > > > > > > Takuya, > > > > > > > > An initialize Server Startup mechanism is cool and useful idea that > > > > shouldn't be too hard to implement. However, I'm wondering why you really > > > > need it. Can you provide a little more details about why you think it is > > > > needed and why static initialization is not adequate. Some code fragments > > > > would also help explain the situation. > > > > > > > > thanks, aaron > > > > > > > > At 10:32 PM 4/20/2005, you wrote: > > > >> Hi Philip, and hacky hackers. > > > >> > > > >> I am wondering if I or someone can implement the startup interface to > > > >> initialize something at the server startup. > > > >> > > > >> Basically what I want to do is to instatiate ReviewIdGroup manager > > > >> singleton which sets reviewId and Reviewgroup listeners. > > > >> > > > >> Current solution to be able to achieve this is > > > >> > > > >> 1) add this in the org.hackystat.kernel.ServerStartup class > > > >> > > > >> 2) add this in a class which is instantiated and called in a requestHook > > > >> (e.g. in some selector classses) > > > >> > > > >> However, since the sensor data listener should be added before any > > > >> requestHook was called, all I can do is to add my code to the > > > >> ServerStartup class (1). > > > >> > > > >> The 1) would be problem because it's coupled and hackyKernel can not be > > > >> complied without my module. > > > >> > > > >> To avoid this, I would like to create hackystat early startup framework > > > >> to startup the implementing class which might be defined in xml file and > > > >> instantiated as reflection. > > > >> > > > >> The interface could be something like > > > >> > > > >> public Interface HackyStartup { > > > >> > > > >> public earlyStartup(); > > > >> > > > >> > > > >> If we implemnt this the following lines in the > > > >> org.hackystat.kernel.ServerStartup would be gone and less decoupled. > > > >> > > > >> xml file would be > > > >> > > > >> <earlyStartup class="org.hackystat.app.review.StartupImpl"> > > > >> <require class="org.hackystat.kernel.StartupImpl"> <!-- optional --> > > > >> <require class="org.hackystat.stdext.StartupImpl"> <!-- optional --> > > > >> </earyStartup> > > > >> > > > >> What do you think? > > > >> > > > >> takuya > > > >> > > > >> > > > >> ================================ > > > >> Takuya Yamashita > > > >> E-mail: [EMAIL PROTECTED] > > > >> ================================ > > > > > > > >================================ > >Takuya Yamashita > >E-mail: [EMAIL PROTECTED] > >================================ >
================================ Takuya Yamashita E-mail: [EMAIL PROTECTED] ================================
