Hi Carlos, thanks a lot for your feedbacks and suggestion, very ver appreciated!!! Hasta pronto! Simo
http://people.apache.org/~simonetripodi/ On Thu, May 13, 2010 at 1:20 PM, Carlos Vara <[email protected]> wrote: > Hi Simone, > > I took a look at your new interceptor. Looking very nice and clean to > my eyes, good job there! > > Arrivederci (I'm afraid my italian skills are nowhere close to your > spanish ones), > Carlos > > On Tue, May 11, 2010 at 4:46 PM, Simone Tripodi > <[email protected]> wrote: >> Hi all, >> I just integrated Carlos Vara's suggestions (thanks a lot!!!) and >> tests seem working nicely. I'd ask once again a little review and, if >> it is ok for you, I'd start moving the module from the sandbox to the >> top level. Please let me know!!! :) >> Best regards, have a nice day, >> Simo >> >> http://people.apache.org/~simonetripodi/ >> >> >> >> On Mon, May 10, 2010 at 10:37 PM, Donald Woods <[email protected]> wrote: >>> Great review Carlos! >>> >>> Simone, I haven't had time to look thoroughly at the code, but it looks >>> like it uses BVAL correctly and is a good complement module. I was able >>> to build the module and it passed rat:check, so I'd vote for bringing it >>> over into trunk. >>> >>> >>> -Donald >>> >>> >>> On 5/9/10 6:10 PM, Carlos Vara wrote: >>>> Hi Simone, >>>> >>>> I took a look at the code, very good work there! >>>> >>>> I'm afraid I can't comment much about the guice part as I'm more of a >>>> spring >>>> man myself, but I can comment a little on the validation part. >>>> >>>> First, just to check that I understood what it does right: >>>> - It creates an interceptor for method calls annotated with @Validate and >>>> validates all the arguments. In case any validation error is found, it >>>> throws a CVE. >>>> >>>> If the above is right, a few pointers on the validation part: >>>> >>>> - If you validate the arguments individually, maybe having an extra >>>> annotation in them can help improve the speed of the code. Take care that >>>> calling validator.validate(..) for all basic types (Strings, Integers, >>>> etc.) >>>> will always output an empty set, and those arguments are very common. >>>> You can use @Valid as a marker for the arguments that you want to validate, >>>> this way, a validated method call would be like this: >>>> >>>> @Validate >>>> public String myMethod(String notValidated, @Valid Person validatedPerson) >>>> { >>>> ... } >>>> >>>> You may decide to validate all arguments in case @Validate is present and >>>> no >>>> argument is annotated with @Valid. >>>> >>>> - Although it is not portable, you may also want to take a look at the >>>> method validation API that the Apache implementation has. It works quite >>>> well in this scenario. I posted a quick implementation I did with AspectJ >>>> here: >>>> http://carinae.net/2010/04/automatic-validation-method-calls-with-jsr-303-appendix-c/ >>>> If you are interested, you could do a second interceptor that takes >>>> advantage of it in case the JSR-303 implementation is the apache one. >>>> >>>> - One last thing, in ValidateMethodInterceptor.invoke(), you may want to >>>> also validate the return value before returning. >>>> >>>> Oh, and my non-binding opinion about moving it out of the sandbox would be >>>> a >>>> yes. All the above comments are just possible improvements, IMHO code uses >>>> the validation API correctly to solve an interesting problem. >>>> >>>> Regards, >>>> Carlos >>>> >>>> >>>> >>>> On Sat, May 8, 2010 at 1:07 PM, Simone Tripodi >>>> <[email protected]>wrote: >>>> >>>>> Hi all guys, >>>>> I'd like to ask you what you think about moving the actual google >>>>> guice integration support from sandbox to current modules. >>>>> I just integrated the groups support in the AOP interceptor and it >>>>> seems working nicely, at least for me, I'd really would like if >>>>> someone could provide feedbacks. >>>>> Every kind of help will be more than appreciated, thanks in advance! >>>>> Simo >>>>> >>>>> http://people.apache.org/~simonetripodi/<http://people.apache.org/%7Esimonetripodi/> >>>>> >>>> >>> >> >
