Is this a good practice to use the "default" Java access to hide classes and methods from client

Question

• In the case of classes:

If we use the factory method we'll have to return created implementation as the type of an implemented interface.

public class Factory {

  public Product getProduct() {
    return new ProductA();
  }
}

---

public interface Product {
}

---

class ProductA implements Product {
}

To avoid client's ability to cast returned Product to concrete implementation of the Product{A, B, C... etc.} we have to:

1. package client's and factory's code separately (let's say com.example.client and com.example.factory)
2. declare concrete implemantations with the default ("package") access (visible to Factory, not visible to Client)

---

    package com.example.client;
    ...
    public class Client {
      public static void main(String[] args) {
        Product i = new Factory().getProduct();
        ProductA a = (ProductA) i; // the type of ProductA isn't visible.
      }
    }

• In the case of methods:

For example we need to use the same factory with the hidden method

public class Factory {

  public Product getProduct() {
    return new ProductA();
  }

  Product[] getCreatedProducts() {
    ...
  }
}

---

I see two problems here:

• bad package structure: hidden classes and methods must be in one package with the calling code.
• bad code: less intuitive and understandable. It's easy to break with the replacement of java files to another package.

Answer 1

The "default" access does not guarantee much of anything, since any rogue programmer can declare their class in your package. Also, regardless of your package structure, in java, you almost always can do an "instance of" check, and then downcast to the "instance of" type. So, if your goal is to prevent any downcasting whatsoever, you must use the private keyword. For example, you can declare the concrete implementations of your Product interface as private static or as anonymous inner classes within your Factory. Indeed, in Bloch's "How to design a good API" article, he makes a point that you should "Minimize Accessibility of Everything."

That said, I think you're being a little paranoid here. Does it really matter that much to you if somebody downcasts? Any code that you write can be misused, and certainly if you include a well-documented factory then you have provided clear information about how to use your API properly. Also, if you build a real factory method that takes arguments and has clear method names, as opposed to this toy Factory example that takes no arguments, then I think you'll find that you're broadcasting the publicly relevant part of what's being created anyway.

Answer 2

I do not really understand why do you want to put factory and classes to separate packages.

I usually create public interface, public factory class and package protected implementations in the same package. So client can create instances using factory only and cannot down cast because the concrete classes are not visible from other package.

Answer 3

In your case here, you have the client knows the factory which knows the implementation class. If they are all in the same process, then both the client and the implementation class are loaded into the same process, which means that the client can have access to the underlying methods of the implementation class via reflection. This assumes that you do not have complete control over the client runtime, i.e. taking measures to prevent reflection. However, if you did, then you probably wouldn't need to worry about the inability of the client to cast to the implementation class.

So, if you view this as a potential security mechanism against an untrusted client process, then I wouldn't put any faith in it. If you have control over the client, then this is probably good enough to keep errant programmers from making an unintentional mess.

Answer 4

I do not see the advantage of two packages. I suggest this alternative:

    package com.example.client ;
    public interface Product
    {
         /* stuff */
    }

    package com.example.client ;
    public interface ProductFactory
    {
         Product make ( X1 x1 , X2 x2 , /* parameters */ , Xn xn ) ;
    }

    package com.example.manager;
    interface ManagedProduct extends com.example.client.Product
    {
         /* management methods */
    }

    package com.example.manager ;
    public final class DefaultProductFactory implements com.example.client.ProductFactory
    {
         public static final DefaultProductFactory instance = new DefaultProductFactory ( ) ;

         private DefaultProductFactory ( )
         {
              super ( ) ;
         }

         public ManagedProduct make ( final X1 x1 , final X2 x2 , /* parameters */ , final Xn xn )
         {
               return new ManagedProduct ( )
               {
                    /* implementation logic here */
               } ;
         }

         /*
              possibly other methods
              The Product implementation class is invisible.
          */
    }

1. Using two packages unnecessarily exposes the implementation Product class to the com.example.manager.DefaultProductFactory class. I would argue that my approach is superior to Bringer128's private inner class Factory. With my approach, the implementation Product class is even invisible to other methods that may exist in the implementation Factory class.
2. If you make the parameters final, then you can use them in the implementation Product class directly from the method arguments (no need to (1) create X1 x1, X2 x2, ..., Xn xn members; (2) this.x1=x1, this.x2=x2, ..., and this.xn=xn in the constructor; and (3) invoke the constructor with ProductImpl (x1,x2,...,xn). This is admittedly small but it saves you keystrokes.
3. I strongly agree with philwb. This should not be regarded as security.
4. This allows classes in com.example.manager to have more methods on the same object than classes in other packages - as requested in Is this a good practice to use the "default" Java access to hide classes and methods from client.