So, this is my design. AccessDecorator classes have a reference to another Access just like normal Decorator Pattern.

design

But the problem is when I create an AccessDecorator wrapping a ConcreteAccess and then try to see which type the Access is:

Access access = new InAllowedAccess();
Access wrapperAccess = new MismatchAccess(access); 
if (wrapperAccess instanceof InAllowedAccess)   //this condition could be used to be a predicate for a filtering over an access list for example
    //do something

Of course this won't work because wrapperAccess is not of type InAllowedAccess but what I really want to know is the type of the concrete access wrapped by the decorator.

I thought about implementing methods like isInstanceofInAllowed(), isInstanceofOutAllowed(), isInstanceofInDenied() and isinstanceofOutDeniedd() in Access classes but don't seems a good solution, I don't know...

Otherwise should I create decorator classes for each 4 types InAllowedAccess, OutAllowedAccess, InDeniedAccess and OutDeniedAccess?

Or there is other better design?

2 Answers

0
Community On

Avoiding type checking is the usually the best way to do things. Unfortunately you haven't given enough context how you are going to use your classes so that I can give an example on how you can use polymorphism and avoid it.

Adding type checking will limit the ability of your system to grow because as new classes are added, these types need to be included in your type checks. Sometimes this can lead to bugs as your code can make assumptions of the number of classes or their types. Here's an example:

Note: I just made this up for illustrational purpose. It's not about having to represent your logic or anything like that.

public void someMethod(Access access) {

    if(access instance of InAccess) { 
        InAccess inAccess = (InAccess)access;
    }
    else {
        OutAccess outAccess = (OutAccess)access;
    }
}

When we started our system had two classes that inherit from Access. Assume that we add another Access class to our system. This code will crash on the else because we may pass the new third access type and the cast won't succeed.

Of course this isn't always the case. Sometimes the number of classes that you have won't grow too much. It's possible that you can predict all types that will have.

And of course, since all things can happen in programming, sometimes you do need to know the types of objects you are using.

Let's assume that your system do need to know the type of objects. Here are two solutions:

  1. Add an enum that will represent all types that you have.

public enum AccessType { InAccessAllowed, InAccessDenied, OutAccessDenied, // other types }

public interface Access {
   AccessType getType();
   // other stuff
}

This way you will use the enum AccessType instead of type casting.

  1. Use interfaces.

Instead of using classes define an interface for each type of Access. Then you will check for the interfaces instead of classes. This way your decorators can implement the same interface as the class it decorates.

public interface InAllowedAccess { }

public class InAllowedAccessImp implements InAllowedAccess { }

public class InAllowedAccessDecorator implements InAllowedAccess { }

I just wan't give an example of an alternative implementations. Since context is lacking in your description, I'll just try to guess how you are going to use your classes and add behavior to them. It's just an idea an nothing more.

Let's assume that your system grant access to users. Users can be given In and Out access and some part of your system need to ask if an access is granted or denied to a specific user so that it can execute a specific logic.

If you don't have any behavior associated with your Access classes you can just use it as a descriptor that will carry the information necessary for other classes to do their jobs.

public enum PortType { In, Out }
public enum Permissions { Allowed, Denied }

public class Access {
    private PortType mPortType;
    private Permissions mPermissions;

    public Access(PortType portType, Permissons permissions) {
        mPortType = portType;
        mPermissions = permissions;
    }

    public PortType getType() { return mPortType; }
    public Permissions getPermissions() { return mPermissions; }
}

If you do have behavior, then you can use polymorphism. Define the behavior in your Access interface and let classes that impelement this interface define the behavior.

Let's say we have messaging system that a user can receive (In) and send (out) messages. These messages go trough a channel. These channels will either accept or reject messages. Here's a way you can use polymorphism instead of type checking.

public interface MessageChannel {

    public bool canSendMessages(); // out
    public bool canReceiveMessages(); // in

    public void receiveMessage(Message m);
    public void sendMessage(Message m);
}

public class InMessageChannel implements MessageChannel {

    // out messaging is not allowed, cannot send
    public bool canSendMessages() { return false; } 

    // In messaging allowed, can receive
    public bool canReceiveMessages() { return true; } 

    public void sendMessage(Message m) {
        throw new SendingMessagesIsNotAllowd();
    }
    public void receiveMessage(Message m); { 
        // do something with the mssage
    }
}

public class OutMessageChannel implements MessageChannel {

    // out messaging allowed
    public bool canSendMessages() { return true; } 

    // In messaging not allowed
    public bool canReceiveMessages() { return false; } 

    public void sendMessage(Message m) {
        // send the message
    }

    public void receiveMessage(Message m); { 
        throw new ReceivingMessagesIsNotAllowd();
    }
}

As you can see each MessageCahnnel has a behavior accosiated with it. It can either send of receive messages if it's allowed or not. This way other classes that use the MessageChannel won't have to do type casting.

0
Nghia Bui On

I thought about implementing methods like isInstanceofInAllowed(), isInstanceofOutAllowed(), isInstanceofInDenied() and isinstanceofOutDeniedd() in Access classes but don't seems a good solution, I don't know...

You are right. That’s a bad solution. An interface often belongs to a layer with high level of abstraction in software, thus the list of its methods should be stable. If you put such a bunch of methods like above into the Access interface, the interface would be very unstable since in the future it’s very likely that you will add more such methods to it.

The simplest solution to your problem is adding (only one time) a new method named core()to the Access interface. Every decorator just implements this method by returning the wrapped/core object.

interface Access {
    ...
    Access core();
}

Access a = ...
if (a.core() instanceof ...