citu_adrian (17) [Avatar] Offline
Hi again,

Here are my comments about the chapter 11:

Paragraph 11.7, Listing 11.15 Testing a JPA-based repository implementation. You use the @Autowired annotation; I think you should add a small comment about this Spring specific annotation (what does it means, what it is the purpose) or eventually just remove the annotation since it is not important for the example.

Paragraph 11.8 "Listing 11.19 shows an EJB policy-enforcement aspect that..." It is the listing 11.20 smilie

Paragraph 11.5.1 Listing 11.7 An aspect that detects public access to members. The aspect it works but will raise a warning for the enum fields which by definition are public.
I tried to fix the aspect to not include the enum fields but I didn't succeed smilie.

Something like this does not work:

"get(public !final !java.langEnum *) "
+ " || set(public * *)")
public static final String NO_PUBLIC_FIELDS = .....

Paragraph 11.8.2 Listing 11.21 Library aspect for EJB pointcuts. In the explanation of the aspect I think you should add a link to the chapter where the supplying annotation is explained (it is the paragraph 5.4). For the reader is quite handy to go back to "refresh his memory" when some AspectJ feature is used into the aspect.

Well, that's all.


ramnivas (171) [Avatar] Offline
Re: comments about the chapter 11
As usual, you make good suggestions. Thanks.

I will incorporate all this feedback. Good catch on the Enum fields. I will check why your code didn't work and provide a solution.

ramnivas (171) [Avatar] Offline
Re: comments about the chapter 11
I checked the aspect to see how enums fit in. The only change needed is to add !final to the set() pointcut. So the modified aspects looks like:

aspect DetectPublicAccessMembers {
declare warning :
get(public !final * *) || set(public !final * *) :
"Please consider using nonpublic access";

Note that accessing a nonfinal field of an enum type is still an error.

citu_adrian (17) [Avatar] Offline
Re: comments about the chapter 11

I guess you will modify also the comment attached to the aspect to reflect the modifications: "Similarly, the pointcut set(public * *) selects all write access to any public
field of any class. In case of write access, we have omitted !final, because Java’s access
check will take care of issuing an error for writing to a final field."

ramnivas (171) [Avatar] Offline
Re: comments about the chapter 11
Yes, definitely.