Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there an elegant way to make every method in a class start with a certain block of code?

People also ask

How many methods a class should have?

a) Methods should not have more than an average of 30 code lines (not counting line spaces and comments). b) A class should contain an average of less than 30 methods, resulting in up to 900 lines of code.

How many methods of class are there?

There are basically three types of methods in Python: Instance Method. Class Method. Static Method.


I don't know about elegant, but here is a working implementation using Java's built-in java.lang.reflect.Proxy that enforces that all method invocations on Foo begin by checking the enabled state.

main method:

public static void main(String[] args) {
    Foo foo = Foo.newFoo();
    foo.setEnabled(false);
    foo.bar(); // won't print anything.
    foo.setEnabled(true);
    foo.bar(); // prints "Executing method bar"
}

Foo interface:

public interface Foo {
    boolean getEnabled();
    void setEnabled(boolean enable);

    void bar();
    void baz();
    void bat();

    // Needs Java 8 to have this convenience method here.
    static Foo newFoo() {
        FooFactory fooFactory = new FooFactory();
        return fooFactory.makeFoo();
    }
}

FooFactory class:

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;

public class FooFactory {

    public Foo makeFoo() {
        return (Foo) Proxy.newProxyInstance(
                this.getClass().getClassLoader(),
                new Class[]{Foo.class},
                new FooInvocationHandler(new FooImpl()));
    }

    private static class FooImpl implements Foo {
        private boolean enabled = false;

        @Override
        public boolean getEnabled() {
            return this.enabled;
        }

        @Override
        public void setEnabled(boolean enable) {
            this.enabled = enable;
        }

        @Override
        public void bar() {
            System.out.println("Executing method bar");
        }

        @Override
        public void baz() {
            System.out.println("Executing method baz");
        }

        @Override
        public void bat() {
            System.out.println("Executing method bat");
        }

    }

    private static class FooInvocationHandler implements InvocationHandler {

        private FooImpl fooImpl;

        public FooInvocationHandler(FooImpl fooImpl) {
            this.fooImpl = fooImpl;
        }

        @Override
        public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
            if (method.getDeclaringClass() == Foo.class &&
                !method.getName().equals("getEnabled") &&
                !method.getName().equals("setEnabled")) {

                if (!this.fooImpl.getEnabled()) {
                    return null;
                }
            }

            return method.invoke(this.fooImpl, args);
        }
    }
}

As others have pointed out, it does seem like overkill for what you need if you only have a handful of methods to worry about.

That said, there certainly are benefits:

  • A certain separation of concerns is achieved, because Foo's method implementations don't have to worry about the enabled check cross-cutting concern. Instead, the method's code only needs to worry about what the method's primary purpose is, nothing more.
  • There is no way for an innocent developer to add a new method to the Foo class and mistakenly "forget" to add the enabled check. The enabled check behavior is automatically inherited by any newly added method.
  • If you need to add another cross-cutting concern, or if you need to enhance the enabled check, it's very easy to do so safely and in one place.
  • It is kind of nice that you can get this AOP-like behavior with built-in Java functionality. You are not forced into having to integrate some other framework like Spring, though they can definitely be good options too.

To be fair, some of the downsides are:

  • Some of the implementation code that handles the proxy invocations is ugly. Some would also say that having inner classes to prevent instantiation of the FooImpl class is ugly.
  • If you want to add a new method to Foo, you have to make a change in 2 spots: the implementation class and the interface. Not a big deal, but it's still a bit more work.
  • Proxy invocations are not free. There is a certain performance overhead. For general use though, it won't be noticeable. See here for more information.

EDIT:

Fabian Streitel's comment got me thinking about 2 annoyances with my above solution that, I'll admit, I'm not happy about myself:

  1. The invocation handler uses magic strings to skip the "enabled-check" on the "getEnabled" and "setEnabled" methods. This can easily break if the method names are refactored.
  2. If there was a case where new methods need to be added that should not inherit the "enabled-check" behavior, then it can be pretty easy for the developer to get this wrong, and at the very least, it would mean adding more magic strings.

To resolve point #1, and to at least ease the problem with point #2, I would create an annotation BypassCheck (or something similar) that I could use to mark the methods in the Foo interface for which I don't want to perform the "enabled check". This way, I don't need magic strings at all, and it becomes a lot easier for a developer to correctly add a new method in this special case.

Using the annotation solution, the code would look like this:

main method:

public static void main(String[] args) {
    Foo foo = Foo.newFoo();
    foo.setEnabled(false);
    foo.bar(); // won't print anything.
    foo.setEnabled(true);
    foo.bar(); // prints "Executing method bar"
}

BypassCheck annotation:

import java.lang.annotation.*;

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface BypassCheck {
}

Foo interface:

public interface Foo {
    @BypassCheck boolean getEnabled();
    @BypassCheck void setEnabled(boolean enable);

    void bar();
    void baz();
    void bat();

    // Needs Java 8 to have this convenience method here.
    static Foo newFoo() {
        FooFactory fooFactory = new FooFactory();
        return fooFactory.makeFoo();
    }
}

FooFactory class:

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;

public class FooFactory {

    public Foo makeFoo() {
        return (Foo) Proxy.newProxyInstance(
                this.getClass().getClassLoader(),
                new Class[]{Foo.class},
                new FooInvocationHandler(new FooImpl()));
    }

    private static class FooImpl implements Foo {

        private boolean enabled = false;

        @Override
        public boolean getEnabled() {
            return this.enabled;
        }

        @Override
        public void setEnabled(boolean enable) {
            this.enabled = enable;
        }

        @Override
        public void bar() {
            System.out.println("Executing method bar");
        }

        @Override
        public void baz() {
            System.out.println("Executing method baz");
        }

        @Override
        public void bat() {
            System.out.println("Executing method bat");
        }

    }

    private static class FooInvocationHandler implements InvocationHandler {

        private FooImpl fooImpl;

        public FooInvocationHandler(FooImpl fooImpl) {
            this.fooImpl = fooImpl;
        }

        @Override
        public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
            if (method.getDeclaringClass() == Foo.class
                    && !method.isAnnotationPresent(BypassCheck.class) // no magic strings
                    && !this.fooImpl.getEnabled()) {

                return null;
            }

            return method.invoke(this.fooImpl, args);
        }
    }
}

There is a lot of good suggestions.. what you can do to strike your problem is think in the State Pattern and implement it.

Take a look at this code snippet.. perhaps it will get you to an idea. In this scenario looks like you want to modify the entire methods implementation based on the internal state of the object. Please recall that the sum of the methods in a object is knows as behavior.

public class Foo {

      private FooBehaviour currentBehaviour = new FooEnabledBehaviour (); // or disabled, or use a static factory method for getting the default behaviour

      public void bar() {
        currentBehaviour.bar();
      }
      public void baz() {
        currentBehaviour.baz();
      }
      public void bat() {
        currentBehaviour.bat();
      }

      public void setFooEnabled (boolean fooEnabled) { // when you set fooEnabel, you are changing at runtime what implementation will be called.
        if (fooEnabled) {
          currentBehaviour = new FooEnabledBehaviour ();
        } else {
          currentBehaviour = new FooDisabledBehaviour ();
        }
      }

      private interface FooBehaviour {
        public void bar();
        public void baz();
        public void bat();
      }

      // RENEMBER THAT instance method of inner classes can refer directly to instance members defined in its enclosing class
      private class FooEnabledBehaviour implements FooBehaviour {
        public void bar() {
          // do what you want... when is enabled
        }
        public void baz() {}
        public void bat() {}

      }

      private class FooDisabledBehaviour implements FooBehaviour {
        public void bar() {
          // do what you want... when is desibled
        }
        public void baz() {}
        public void bat() {}

      }
}

Hope you like it!

P.D: Is an implementation of the State Pattern (also knows as Strategy depending on the context.. but the principles are just the same).


Yes, but it's a bit of work, so it depends how important it is to you.

You can define the class as an interface, write a delegate implementation, and then use java.lang.reflect.Proxy to implement the interface with methods that do the shared portion and then conditionally call the delegate.

interface Foo {
    public void bar();
    public void baz();
    public void bat();
}

class FooImpl implements Foo {
    public void bar() {
      //... <-- your logic represented by this notation above
    }

    public void baz() {
      //... <-- your logic represented by this notation above
    }

    // and so forth
}

Foo underlying = new FooImpl();
InvocationHandler handler = new MyInvocationHandler(underlying);
Foo f = (Foo) Proxy.newProxyInstance(Foo.class.getClassLoader(),
     new Class[] { Foo.class },
     handler);

Your MyInvocationHandler can look something like this (error handling and class scaffolding omitted, assuming fooIsEnabled is defined somewhere accessible):

public Object invoke(Object proxy, Method method, Object[] args) {
    if (!fooIsEnabled) return null;
    return method.invoke(underlying, args);
}

It's not incredibly pretty. But unlike various commenters, I'd do it, as I think repetition is a more important risk than this kind of density, and you'll be able to produce the "feel" of your real class, with this somewhat inscrutable wrapper added on very locally in just a couple of lines of code.

See the Java documentation for details on dynamic proxy classes.


This question is closely related to aspect-oriented programming. AspectJ is an AOP extension of Java and you may give it a look to get some ispiration.

As far as I know there is no direct support for AOP in Java. There are some GOF patterns that relate to it, like for instance Template Method and Strategy but it will not really save you lines of code.

In Java and most other languages you could define the recurrent logic you need in functions and adopt a so-called disciplined coding approach in which you call them at the right time.

public void checkBalance() {
    checkSomePrecondition();
    ...
    checkSomePostcondition();
}

However this would not fit your case because you would like the factored-out code to be able to return from checkBalance. In languages that support macros (like C/C++) you could define checkSomePrecondition and checkSomePostcondition as macros and they would simply be replaced by the preprocessor before the compiler is even invoked:

#define checkSomePrecondition \
    if (!fooIsEnabled) return;

Java does not have this out of the box. This may offend someone but I did use automatic code generation and template engines to automate repetitive coding tasks in the past. If you process your Java files before compiling them with a suitable preprocessor, for instance Jinja2, you could do something similar to what is possible in C.

Possible pure Java approach

If you are looking for a pure Java solution, what you may find is probably not going to be concise. But, it could still factor out common parts of your program and avoid code duplication and bugs. You could do something like this (it's some sort of Strategy-inspired pattern). Note that in C# and Java 8, and in other languages in which functions are a little easier to handle, this approach may actually look nice.

public interface Code {
    void execute();
}

...

public class Foo {
  private bool fooIsEnabled;

  private void protect(Code c) {
      if (!fooIsEnabled) return;
      c.execute();
  }

  public void bar() {
    protect(new Code {
      public void execute() {
        System.out.println("bar");
      }
    });
  }

  public void baz() {
    protect(new Code {
      public void execute() {
        System.out.println("baz");
      }
    });
  }

  public void bat() {
    protect(new Code {
      public void execute() {
        System.out.println("bat");
      }
    });
  }
}

Kinda of a real-world scenario

You are developing a class to send data frames to an industrial robot. The robot takes time to complete a command. Once the command is completed, it sends you a control frame back. The robot may get damaged if it receives a new command while the previous is still being executed. Your program uses a DataLink class to send and receive frames to and from the robot. You need to protect access to the DataLink instance.

The user interface thread calls RobotController.left, right, up or down when the user clicks the buttons, but also calls BaseController.tick at regular intervals, in order to reenable command forwarding to the private DataLink instance.

interface Code {
    void ready(DataLink dataLink);
}

class BaseController {
    private DataLink mDataLink;
    private boolean mReady = false;
    private Queue<Code> mEnqueued = new LinkedList<Code>();

    public BaseController(DataLink dl) {
        mDataLink = dl;
    }

    protected void protect(Code c) {
        if (mReady) {
            mReady = false;
            c.ready(mDataLink);
        }
        else {
            mEnqueue.add(c);
        }
    }

    public void tick() {
        byte[] frame = mDataLink.readWithTimeout(/* Not more than 50 ms */);

        if (frame != null && /* Check that it's an ACK frame */) {
          if (mEnqueued.isEmpty()) {
              mReady = true;
          }
          else {
              Code c = mEnqueued.remove();
              c.ready(mDataLink);
          }
        }
    }
}

class RobotController extends BaseController {
    public void left(float amount) {
        protect(new Code() { public void ready(DataLink dataLink) {
            dataLink.write(/* Create a byte[] that means 'left' by amount */);
        }});
    }

    public void right(float amount) {
        protect(new Code() { public void ready(DataLink dataLink) {
            dataLink.write(/* Create a byte[] that means 'right' by amount */);
        }});
    }

    public void up(float amount) {
        protect(new Code() { public void ready(DataLink dataLink) {
            dataLink.write(/* Create a byte[] that means 'up' by amount */);
        }});
    }

    public void down(float amount) {
        protect(new Code() { public void ready(DataLink dataLink) {
            dataLink.write(/* Create a byte[] that means 'down' by amount */);
        }});
    }
}

I would consider refactoring. This pattern is heavily breaking DRY pattern (Don't repeat yourself). I believe this break this class responsibility. But this depends on your control of code. Your question is very open - where are you calling Foo instance?

I suppose you have code like

foo.bar(); // does nothing if !fooEnabled
foo.baz(); // does also nothing
foo.bat(); // also

maybe you should call it something like this way:

if (fooEnabled) {
   foo.bat();
   foo.baz();
   ...
}

And keep it clean. For example, logging:

this.logger.debug(createResourceExpensiveDump())

a logger is not asking himself, if debug is enabled. It just logs.

Instead, calling class need to check this:

if (this.logger.isDebugEnabled()) {
   this.logger.debug(createResourceExpensiveDump())
}

If this is a library and you cannot control calling of this class, throw an IllegalStateException which explains why, if this calling is illegal and cause trouble.


IMHO the most elegant and best performing solution to this is to have more than one implementation of Foo, together with a factory method for creating one:

class Foo {
  protected Foo() {
    // Prevent direct instantiation
  }

  public void bar() {
    // Do something
  }

  public static void getFoo() {
    return fooEnabled ? new Foo() : new NopFoo();
  }
}

class NopFoo extends Foo {
  public void bar() {
    // Do nothing
  }
}

Or a variation:

class Foo {
  protected Foo() {
    // Prevent direct instantiation
  }

  public void bar() {
    // Do something
  }

  public static void getFoo() {
    return fooEnabled ? new Foo() : NOP_FOO;
  }

  private static Foo NOP_FOO = new Foo() {
    public void bar() {
      // Do nothing
    }
  };
}

As sstan points out, even better would be to use an interface:

public interface Foo {
  void bar();

  static Foo getFoo() {
    return fooEnabled ? new FooImpl() : new NopFoo();
  }
}

class FooImpl implements Foo {
  FooImpl() {
    // Prevent direct instantiation
  }

  public void bar() {
    // Do something
  }
}

class NopFoo implements Foo {
  NopFoo() {
    // Prevent direct instantiation
  }

  public void bar() {
    // Do nothing
  }
}

Adapt this to the rest of your circumstances (are you creating a new Foo every time or reusing the same instance, etc.)


I have another approach: have a

interface Foo {
  public void bar();
  public void baz();
  public void bat();
}

class FooImpl implements Foo {
  public void bar() {
    //...
  }
  public void baz() {
    //...
  }
  public void bat() {
    //...
  }
}

class NullFoo implements Foo {
  static NullFoo DEFAULT = new NullFoo();
  public void bar() {}
  public void baz() {}
  public void bat() {}
}

}

and then you can do

(isFooEnabled ? foo : NullFoo.DEFAULT).bar();

Maybe you can even replace the isFooEnabled with a Foo variable which either holds the FooImpl to be used or the NullFoo.DEFAULT. Then the call is simpler again:

Foo toBeUsed = isFooEnabled ? foo : NullFoo.DEFAULT;
toBeUsed.bar();
toBeUsed.baz();
toBeUsed.bat();

BTW, this is called the "Null pattern".