-2

I have interface called RestClientInterface which is implemented by abstract RestClient class, those class is extended by SearchRestClient and IndexRestClient. Both of those classes are singletons. I would like to be able implement in the interface static method getInstance. I decided to use Guava library like suggested here: How can I implement abstract static methods in Java?.

I have implemented two statics methods in interface:

public interface RestClientInterface {

    ClassToInstanceMap<RestClientInterface> classToInstanceMap = MutableClassToInstanceMap.create();

    static <T extends RestClientInterface> T getInstance(Class<T> type) {
        return classToInstanceMap.getInstance(type);
    }

    static <T extends RestClientInterface> void registerInstance(Class<T> type, T identity) {
        classToInstanceMap.putIfAbsent(type, identity);
    }
}

And next registered instances in both extending classes:

public class IndexRestClient extends RestClient {

    static {
        RestClientInterface.registerInstance(IndexRestClient.class, getInstance());
    }

    /**
     * IndexRestClient singleton.
     */
    private static IndexRestClient instance = null;

    private IndexRestClient() {}

    /**
     * Return singleton variable of type IndexRestClient.
     *
     * @return unique instance of IndexRestClient
     */
    private static IndexRestClient getInstance() {
        if (Objects.equals(instance, null)) {
            synchronized (IndexRestClient.class) {
                if (Objects.equals(instance, null)) {
                    instance = new IndexRestClient();
                    instance.initiateClient();
                }
            }
        }
        return instance;
   } 
}

Next I call this like that:

IndexRestClient restClient = RestClientInterface.getInstance(IndexRestClient.class);

But every time I get null. Static registration of the instances doesn't work as the array of registered instances is empty. How can I correctly instantiate those both classes?

Beacze
  • 534
  • 3
  • 8
  • 24
  • 1
    It might be that the `IndexRestClient` class is not initialised if solely referenced in the statement `RestClientInterface.getInstance(IndexRestClient.class)` before further use. Before this call, may be try to force class loading with `IndexRestClient.getInstance()` (visibility of the method would need to be increased) - or any other call which would guarantee the class is loaded? – Alexandre Dupriez Nov 01 '17 at 09:33
  • Yes, if I will make getInstance public, I instantiate this class but after that I actually don't need anymore to call RestClientInterface.getInstance(IndexRestClient.class). – Beacze Nov 01 '17 at 09:46
  • I see. From a design perspective, what is the motivation to have the registry, since you already know the concrete type of the client you want to get? That is, when you call `RestClientInterface.getInstance(IndexRestClient.class)`, you already know you wish to use `IndexRestClient`. What is the reason not to expose `IndexRestClient.getInstance()`? You may want to hide the details of the instantiation, which I can understand - – Alexandre Dupriez Nov 01 '17 at 10:28
  • The main idea behind it is that every class which implements RestClientInterface should be a Singleton. The best would be if getInstance with the whole logic (safe thread) would be implemented only once in interface but this is probably not possible. – Beacze Nov 01 '17 at 11:26

2 Answers2

2

Following our discussion, the problem is probably related to the lazy loading of the class IndexRestClient if it is solely referenced in the statement RestClientInterface.getInstance(IndexRestClient.class) before further use, because I am not sure the reference to IndexRestClient.class is enough to trigger the load/initialization of the class.

Assuming the problem here is indeed about the class IndexRestClient being lazily loaded, you would need to invert the control of the registration logic, i.e. instead of the IndexRestClient registering itself with the registry, have a "Registrar" in the middle which takes care of it.

However it seems to me that the contract of your RestClientInterface should be changed since it is not agnostic of the concrete types of your Rest clients while I understand you want to hide the implementation of their creation.

You may want to have a look at the Java service loader mechanism which seems close to what you want to do, e.g.:

public final RestClients {
    private final ServiceLoader<RestClient> restClients = ServiceLoader.load(RestClient.class);

    public RestClient getClient(RestClientSpec spec) throws NoClientFoundForSpecException {
        for (RestClient client : restClients) {
            if (/* client matches your specification */) {
                return client;
            }
        }
        throw new NoClientFoundForSpecException(spec);
    }
}

The Service Loader pattern may provide more encapsulation and hide more of the implementation than you actually want (since you are apparently using concrete types), yet I felt it was worth mentioning.

Alexandre Dupriez
  • 3,026
  • 20
  • 25
-1

That's because the classToInstanceMap object is not static, while the methods you use are static. This means the map is created only when you create a new instance of the class. Simply add the static keyword to this object.

Stav Saad
  • 589
  • 1
  • 4
  • 13
  • I have added it, but IDE says that static modifier is redundant for interface and result is exactly the same, array has size 0. In debug mode it never reaches the point when registerInstance method should be called. – Beacze Nov 01 '17 at 09:26
  • @Beacze The IDE warns you because it is unaware of the way Guava operates, which is somewhat unconventional in Java. Have you tried running it regardless of the warning? – Stav Saad Nov 01 '17 at 09:33
  • Yes, I have run it and still array is empty. – Beacze Nov 01 '17 at 09:35
  • Ill refer you to what @Alexander Dupriez wrote in comment to your question.he might be correct there – Stav Saad Nov 01 '17 at 09:38
  • Yes, calling IndexRestClient.getInstance before makes it work but after that I actually don't need anymore this array and getInstance in interface class. The whole idea was to instantiate it by interface. – Beacze Nov 01 '17 at 09:50