[aspectj-dev] bug in example on website

["Gregor Kiczales" <gregor@xxxxxxxxx>]

There's a bid of bad coding in one of the website examples. The code caused some people to get confused. Unfortunately it caused
people to observe that better code was possible, and that it must be hard to understand AspectJ because even the website had this
code. *sigh* (as Eric might say)

The code is from the Bean example. The BoundPoint aspect CURRENTLY has:

  pointcut setter(Point p): call(void Point.set*(*)) && target(p);

  void around(Point p): setter(p) {
        String propertyName =
           thisJoinPointStaticPart.getSignature().getName().substring("set".length());
        int oldX = p.getX();
        int oldY = p.getY();
        proceed(p);
        if (propertyName.equals("X")){
          firePropertyChange(p, propertyName, oldX, p.getX());
        } else {
          firePropertyChange(p, propertyName, oldY, p.getY());
        }
  }

The problem with this code is that it looks more generic than it is, so its confusing, and it pays a performance penalty for that
too. I think it should be:

  void around(Point p):  call(void Point.setX(int)) {
        int oldX = p.getX();
        proceed(p);
        firePropertyChange(p, "X", oldX, p.getX());
  }
  
  <and the same for y>

Notes:

 - there's no use in a named pointcut here. Overuse of named pointcuts in examples actually causes a surprisingly large number of
people to believe that you HAVE to use named pointcuts in advice

 - the two *s in  set*(*) suggest that this code will work for any property of the point. The propertyName code suggests that as
well. but the explicit calls to getX and getY hardwire the code for X and Y. So it really isn't generic. Given that, the simpler
code is more clear and equally powerful. Its also faster because it doesn't have the string consing.