views:

82

answers:

2

What if I have few factory methods returning non-public type and pairing set of methods which gives variables of this non-public type? This results with titled warning message in NetBeans.

In result public API will contain only two pairing sets of methods. The reason is to make my type hierarchy sealed (like seald classes in Scala) and allow users only instantiate these types through factory methods. So we get DSL in some sense.

For example, Schedule class represented by calendar fields' contraints. There are some types of contraints - Range, Singleton, List, FullSet - with NumberSet interface as a root. We don't want to expose these types and how Schedule interact with them. We just want the specification from the user. So we make NumberSet package-private. In class Schedule we create few factory-methods for constraints:

NumberSet singleton(int value);
NumberSet range(int form, int to);
NumberSet list(NumberSet ... components);

and some methods for creating Schedule object:

Schedule everyHour(NumberSet minutes);
Schedule everyDay(NumberSet minutes, NumberSet hours);

User can only use them in the manner:

Schedule s = Schedule.everyDay( singleton(0), list(range(10-15), singleton(8)) );

Is it bad idea?

+2  A: 

The idea itself is sound. But it won't work, if you make the root type (here: NumberSet) package private. Either expose that type or use a public interface instead. The actual implementation types should (and can) be hidden.

public abstract class NumberSet {

    // Constructor is package private, so no new classes can be derived from
    // this guy outside of its package.
    NumberSet() {
    }
}

public class Factories {

    public NumberSet range(int start, int length) {
        return new RangeNumberSet(start, length);
    }

    // ...
}

class RangeNumberSet extends NumberSet {
   // ... must be defined in the same package as NumberSet
   // Is "invisible" to client code
}

Edit To expose a hidden/private root type from a public API is a mistake. Consider the following scenario:

package example;

class Bar {
    public void doSomething() {
            // ...
    }
}

public class Foo {

    public Bar newBar() {
        return new Bar();
    }
}

and consider a client application using this API. What can the client do? It cannot properly declare a variable to have type Bar since that type is invisible to any class outside of the package example. It cannot even call public methods on a Bar instance it got from somewhere, since it does not know, that such a public method exists (it cannot see the class, let alone any members it exposes). So, the best a client can do here is something like:

Object bar = foo.newBar();

which is essentially useless. A different thing would be to have a public interface (or abstract class) instead of the package private one, as in the code defined above. In this case, the client actually can declare a variable of type NumberSet. It cannot create its own instances or derive subclasses, since the constructor is hidden from it, but it can access the public API defined.

Edit again Even if you want a "featureless" value (from the client's perspective), i.e., a value, which does not define any interesting APIs the client may want to call, it is still a good idea to expose a public base type in the way described above. And be it only for the purpose of the compiler being able to perform type checking and allowing client code to temporarily store such a value into a (properly declared) variable.

If you don't want your client to call any API methods on the type: that's fine. There is nothing preventing you from not providing a public API on your (otherwise) public base type. Just don't declare any. Use an "empty" abstract base class (empty from the perspective of the client, since all interesting methods would be package private and thus hidden). But you have to supply a public base type nonetheless, or you should use plain Object as return value, but then you forfeit error checking at compile time.

Rule of thumb: If the client has to call some method in order to obtain a value and pass it on to some other API, then the client actually knows, that there are some magic special values. And it has to be able to handle it in some way ("pass it on", at very least). Not providing a proper type for the client to deal with the values does not buy you anything except for (entirely appropriate) compiler warnings.

public abstract class Specification {

    Specification() {
        // Package private, thus not accessible to the client
        // No subclassing possible
    }

    Stuff getInternalValue1() {
        // Package private, thus not accessible to the client
        // Client cannot call this
    }
}

The above class is "empty" as far as the client code is concerned; it does not offer an usable API except stuff, which the Object already offers. The major benefit of having it: the client can declare variables of this type, and the compiler is able to type check. Your framework, though, remains the only place, where concrete instances of this type can be created, and thus, your framework has total control over all values.

Dirk
Why won't it work, if we make the root type package-private?
feelgood
Yes, but it's unwanted client to see this type. We just need specification through factory-methods and passing it through factory-methods for Schedule objects.
feelgood
A: 

I can think of two ways to do this, but neither really work:

  1. Declare NumberSet as package private, but change the public API methods that currently use the NumberSet type to use Object instead, and have the implementations cast arguments from Object to NumberSet as required. This relies on callers always passing the right kind of object, and will be prone to ClassCastExceptions.

  2. Implement a public NumberSet interface with no methods and package private InternalNumberSet that implements NumberSet and defines the methods required internally. Type casting is used once again to cast arguments NumberSet to InternalNumberSet. This is better the previous approach, but if someone creates a class that implements NumberSet without also implementing InternalNumberSet, the result will be ClassCastExceptions once again.

I personally think that you should just declare the NumberSet interface as public. There is no real harm in exposing getters in the interface, and if you want your implementation classes as immutable, declare them as final and don't expose the setters.

Stephen C