To me it looks like in your 'args()' argument binding sample pointcuts the syntax is wrong.

Comment From: pivotal-issuemaster

@kriegaex Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

Comment From: pivotal-issuemaster

@kriegaex Thank you for signing the Contributor License Agreement!

Comment From: sbrannen

Thanks for the PR.

It turns out that the examples are actually correct, since they are using a shared common pointcut definition.

Comment From: kriegaex

You are right. I did not notice because I did not read the chapter above when looking for reference information about args(). IMO, this is kind of self-obfuscating. Instead of showing examples with basic syntax, you use something declared in a previous chapter without linking to it.

I am very experienced in AspectJ, also in the trickier parts of the syntax. I do not need the manual for myself, being an AspectJ guy (not really a Spring user). I sometimes open the Spring manual in order to find explanations to link to when answering questions on StackOverflow. I did not expect you to use the rather exotic case of a pointcut not declared in the same class, so you have to refer to it via fully qualified name. Thus, it looks like an oops in a method execution pointcut. You really should not expect all users to read the whole manual in one single session and then remember something from a few screens further above. How did I find the chapter? I was looking for the string "args(" on the opened web page in order to find the target chapter I could link to from StackOverflow, as I said.

Please consider to either change pointcut naming to something ugly, but obvious like pcWhateverItDoes or pointcutWhateverItDoes, then the intent would be clear. This is not "clean code" style naming, but in a reference manual you want to communicate your intent more clearly. Otherwise, explain explicitly that you are referring to a pointcut declared above and put a link to there into the explanation. If it was one long code block, the case would be different. But it is prose interspersed with little snippets. Without lots of scrolling up and down, the user's chances of understanding that part of the manual asymptotically approaches zero.

Comment From: sbrannen

Thanks for providing such valid criticism.

I agree that it can be confusing if you don't read the whole chapter.

I think I'll just rename SystemArchitecture to SharedPointcuts, CommonPointcuts, or something similar.

What do you think about that?

By the way, you already inspired me to make the changes in 52c2ca610b6f583f5ab2a068240e4f10cc279a9a, before your additional comments.

Comment From: kriegaex

I think I'll just rename SystemArchitecture to SharedPointcuts, CommonPointcuts, or something similar.

What do you think about that?

I think that is a particularly good idea, much better than mine to rename the pointcut methods themselves. SharedPointcuts would reflect the wording in the manual. Actually, the corresponding sub-chapter's headline is even "shared common pointcut definitions" which sounds kind of redundant, so maybe better not SharedCommonPointcuts. Even SystemArchitectureSharedPointcuts would be acceptable, covering both the domain language and the AOP concept. But maybe that would be kind of too verbose. Either way, the intent will be conveyed well with both of your suggestions.

Thank you very much for the swift and constructive reaction. 😀

Comment From: sbrannen

Thank you very much for the swift and constructive reaction. 😀

You're very welcome.

Changes made in 8be2a43d527687e7568dc86fee6b11bee994c5e9.