Design decision: extending an interface vs. new interface

Hallo,

I have a little design decision today: There is an existing interface, called 'TargetSystem' which only have one method 'getName()'. There are no other common information about these target systems. Now I have a new kind of target system开发者_高级运维s that need authentication.

I have to know whether a target system needs authentication or not (the frontend have to show a password dialog for those). If it needs authentication, I have to set the username and password.

My design decision: Should I extend the existing interface with methods 'needsAuthentication' and 'setUsernameAndPassword' or creating a new interface extending the old one with only the method 'setUsernameAndPassword', getting the authentication need by instanceof.

Important: There is no need to be downwards compatible or any other reason not to touch the old interface! I just discussing with a co-worker, which way is generally the nice one: creating interfaces with names like 'ObjectWithFeatureX', 'ObjectWithFeatureY' or creating methods like 'hasFeatureX', 'hasFeatureY'.

I don't really agree with with Peter. Sometimes, the instanceof can even play a central role in the design.

Personnaly, I love the following pattern (flameproof suit: "on"):

interface Authentifiable {
    void authentify(...)
}

interface Stateful {
    void saveState(...)
    void loadState(...)
}

interface MyOtherAspect {
   ...
}

And then, in the code:

void someCode()
{
  for (Server s : servers)
  {
    if (s instanceof Authentifiable)
       ((Authentifiable) s).authentify(...)
    if (s instanceof Stateful)
       ((Stateful) s).load(...)
    ...
  }

  for (GridSystem gs : grids)
  {
    if (gs instanceof Authentifiable)
       ((Authentifiable) gs).authentify(...)
    if (gs instanceof Stateful)
       ((Stateful) gs).load(...)
    ...
  }
}

This enables you to have completely orthogonal "aspects" working on any object. You can have object implementing feature A & B, others B & C and others A & C. ...or any combination of any features. If you have many such features, this comes in particularly handy. Making one big interface for all of them where implementing objects just handle all these features with empty stubs might be ugly.

Plus, here, you can check whether a particular object has a particular feature, which you can use directly, for example to split a list of objects into two bunches, one with feature X and the other without, in order to process them differently.

The parts of the system operating on these features simply need to check if the object has properties A, B or C by checking with instanceof. This is scalable, backward compatible and easy.

That said, it is a very specific way for handling things and not necessarily suited for general purpose stuff. It is especially suited if you have a lot of orthogonal features applying on several distinct objects.

Ask a question yourself: AuthenticationSystem is-a TargetSystem?

A solution without downcast:

interface TargetSystem{
   //Each TargetSystem needs a sort of authentication anyway
   boolean authentication(AuthenticationContext context);
   ...
}

class NormalTargetSystem implements TargetSystem{
   boolean authentication(AuthenticationContext context){
       //dummy authentication
       return true;
   }
   ...
}
class AuthenticationTargetSystem implements TargetSystem{
   boolean authentication(AuthenticationContext context){
       //real authentication
   }
   ...
}

In general, if you have a a good design you don't need instanceof.

IMHO: instanecof should only be used for classes/interfaces you cannot change.

Can you have just setUsernameAndPassword() and the implementations which don't need it just ignore it? A more common approach would be to have setUsername() and setPassword() (however I prefer the all in one method approach as it doesn't make much sense to change just one)

creating interfaces with names like 'ObjectWithFeatureX', 'ObjectWithFeatureY' or creating methods like 'hasFeatureX', 'hasFeatureY'.

I would say neither. ;) Where ever possible the caller should not have code like

if(a instanceof NeedsUsername) {
    ((NeedsUsername) a).setUsername(username);
}

or

if(a.needUsername()) {
    a.setUsername(username);
}

it should just have

a.setUsername(username);

EDIT: You need some sort of listener for events such as failed passwords. You could have a listener like

public interface AuthenticationListener {
   public void firstUsernamePassword();
   public void failedAuthentication(String reason);
}

which way is generally the nice one: creating interfaces with names like 'ObjectWithFeatureX', 'ObjectWithFeatureY' or creating methods like 'hasFeatureX', 'hasFeatureY'.

Another question you might ask yourself is about the future plans. Do you plan to have even more features? If you can see the possibility of one day having ObjectWithFeatureXAndFeatureY, you might wish to consider the Decorator design pattern.

This example of adding multiple features like scrollbars to windows show the good use of decorator pattern. http://en.wikipedia.org/wiki/Decorator_pattern#Motivation

Just be careful not to overdesign if you are sure you will never need this much functionality and can just use simple inheritance.

You can always refactor to a system with no checks, but often this conflicts with a nice tiered approach.

Eg: in your case, if you want to show a login dialog only if the target system requires it, you could have an interface method init() which shows the authentication dialog in one case and does nothing in the other. But then you're mixing gui code in your target system, which is not what you want. You can start hassling aroudn with callbacks and all, but in the end, there's no simple way around it.

So here's one approach I like

public interface Authenticating {
  void authenticate(String username, String password);
}

public interface TargetSystem {
  String getName();

  /**
  * @return the authentication interface of this object, or 
  * null if authentication is not required.
  */
  Authenticating getAuthenticationInterface();
}

...

   Authenticating auth = targetSystem.getAuthenticationInterface();
   if (auth!=null) {
     String user = null;
     String pass = null;
     // show login dialog and get response
     auth.authenticate(user, pass);
   }

...

This way, you can only call the authenticate method if it is needed. You might need to think of better names though :)

In this case, I would not let one interface extend the other. The target system can implement both interfaces and just return itself, or it could return an anonymous inner class. But that's completely up to its implementor.