For those who are interested: a bit of a "API design: first impressions"-topic

Hi community,

ATM I’m struggeling with the DefaultServer and KernelAdapter design, while I’m trying to integrate some SSLSocketFactory-involving custom SslSelectorKernel and stuff. Being a total JME-noob I’m currently unsure where to put some feedback

I would like to provide some API/architecture feedback and I hope to not piss off somebody, as empathy and social behaviour aren’t my strength. So, I would like to first thank you all for creating JME and keeping it alive and tell you about my API-user-experience:

  • addChannel(int port) seems to be the intended way to add another channel to my server, okay, fine, that’s what I need
  • addChannel(int port) uses kernelFactory.createKernel(channeIndex, port) for instantiating Kernel - perfect, let’s just pass in my own factory…
  • …but no constructor argument and no setter for passing my desired factory in? Strange…

Checking the DefaultServer source code, tfw seeing this:

private final KernelFactory kernelFactory = KernelFactory.DEFAULT;

Why is this class member final?

An “immutable default factory” is an oxymoron: factories are “exchangeable objects for flexible instantiations”. If you remove “exchangeable”, you also lose “flexible”.

Okay, next I checked the KernelFactory.DEFAULT-implementation: it turns out it’s implementation, NioKernelFactory, always returns a new SelectorKernel-instance. This is a bit odd, because in this case we have a factory which nobody can replace, always returning instances of the same class.

That’s where I stopped poking around for my implementation any further and started checking some more details about the API/code and just wanted to leave some feedback about the architecture:

  1. by introducing factories you (normally) agree that API users can pass in their own factory. I can imagine why you made it immutable/idiot-safe, regarding to your “default server always requires reliable and maybe/optional unreliable channel”, but either it’s an exchangeable factory returning case-dependent object instances, or it’s something immutable/unchangeable always returning instances of one and the same class.
  2. if you introduce factories, then use them consequentially: imagine KernelFactory to be KernelFactory<Kernel>, and use the factory everywhere, where you need to instantiate a new instance of Kernel. That means, kick that DefaultServer constructor-arguments for Kernel reliable and Kernel fast, and require the API user to pass in a KernelFactory instead, which you then use to instantiate Kernels.
  3. the redundant private static final declarations of CH_RELIABLE, CH_UNRELIABLE and CH_FIRST, in DefaultServer and DefaultClient offends the DRY-principle and seems to be no good design decision: as I’m forced to extend from DefaultServer for using implement my own CustomKernelFactory, I now need to declare them a third time for no obvious reason. If both, server and client, need the same constants, you should follow DRY and supply them in a public, transparent and non-redundant way
  4. for me, the choice of hard-coded int-based “channel ID constants” seem to rise from stopping “one abstraction layer too early”. A much cleaner way, in respect of problem described in 3.), would be to use a builder, which routes some channel-names using strings (more self-explanatory and non-colliding than int) to some unique (port/channel) number, and in the end, the builder creates an effectively immutable ChannelMap<String,Integer>.

Maybe the private static final int-declarations are a result of some “maximum performance desire”, but in my opinion some more generic way wouldn’t impact performance that bad, because we’re talking about IO-operations which take way longer than a map.get(channelName) for obtaining some integer to be used in your API.

Let me show some exemplaric code that demonstrates a bit what I just critizised, and which would have made my task, using your API to implement my requirements much easier and faster. That’s a subjective impression, I know. The following code would be split up in a lot of different class-files, but I’m lazy, so I just dump them in here:

// YOU follow the DRY principle and clearly state some public API constants,
// which have the same, pretty speaking names:
public class DefaultChannel {
    // effectively replaces your CH_RELIABLE and CH_UNRELIABLE:
    public static final String RELIABLE = "RELIABLE";
    public static final String UNRELIABLE = "UNRELIABLE";

// that would encourage me, the user of your API principles, to create:
public class SslSecuredChannel {
    // some of **my** project specific stuff:
    public static final String SECURE = "SECURE";

// in order to use the string-based names, we have to change the contract "a bit":
public interface KernelFactory {
    // improve later code readability by switching to String instead of int,
    // because channelName="SECURE" is much more self-explaining than channelName=2:
    public Kernel createKernel(String channelName, int port) throws IOException;

// next, we use real factory to completely handle all the DefaultServers
// Kernel-instantiations:
public class DefaultKernelFactory implements KernelFactory {
    public Kernel createKernel(String channelName, int port) throws IOException {
        if (DefaultChannel.RELIABLE.equals(channelName)) {
            return new SelectorKernel(port);

        if (DefaultChannel.UNRELIABLE.equals(channelName)) {
            return new UdpKernel(port);

        throw new RuntimeException(String.format("Dafuq is '%s'?", channelName));

// that allows me to reuse YOUR efford/code:
public class SecureDefaultKernelFactory extends DefaultKernelFactory {
    public Kernel createKernel(String channelName, int port) throws IOException {
        if (SslSecuredChannel.SECURE.equals(channelName)) {
            return new SslSelectorKernel(port);

        return super.createKernel(channelName, port);

// and enables me, to use a "ChannelSetup"-class which I can use for client and
// server purposes, associating channel names with zerobased indices and port numbers:
public class MyGameChannelSetup {
    public static ChannelMap createChannelMap(ServerArguments args) {
        return ChannelMap.createEmptyBuilder()
                         .route(DefaultChannel.RELIABLE).to(args.getPortTcp()).reliably()          // index=0
                         .route(DefaultChannel.UNRELIABLE).to(args.getPortUdp()).unreliably()      // index=1
                         .route(SslSecuredChannel.SECURE).to(args.getPortSsl()).reliably()         // index=2

// which would result in the following server constuction:
DefaultServer server = new DefaultServer(
                                new SecureDefaultKernelFactory());

// and finally, the DefaultServer-constructor would work like this:
public DefaultServer (String gameName, int gameVersion, ChannelMap channelMap, KernelFactory factory) {
    this.gameName = gameName;
    this.version = gameVersion; = new HostedServiceManager(this);        


    for (String channelName : channelMap.getChannels()) {   // relies on side-effect: getChannels() need
        Kernel kernel = factory.createKernel(               // to return the String-names "in order of addition"

                new KernelAdapter(this, kernel, dispatcher, channelMap.isChannelReliable(channelName)));

I know that there are a lot more required refactorings in order to make that work completely, but I just wanted to provide some feedback on how the overall “first API design impression” I got.

Would love to hear your opinions or background information,

Edits: kicked a misplaced “not” int “not offends the DRY…” and corrected a mismatched reference from “in respect of problem described in 2.” to in “respect of problem described in 3.)”, (and then added this comment)


DefaultServer is the default server implementation.

The idea was that if you didn’t want a default server then you’d make your own server class. For good or ill, that was the intent.

DefaultServer is mostly just connecting up other reusable classes. And while it’s sprawled a bit into “lots of code” territory, it’s still mostly true.

Anyway, if we wanted to add another channel type to the default implementation then we’d do the minimum to add another channel type. DefaultServer is supposed to be as simple as possible and we never meant to be the 'end of all flexibility" as far as channel setup goes. For example, if we add an SSL channel to default server then it would probably be setup like the other two main channels, leaving the addChannel() channels still regular TCP.

Adding flexible user-specific channel configuration is tough because the server sends the configuration to the clients. So the ability to do so will significantly complicate these classes for that 0.1% of users who will actually use it. (And most of those will probably find 100 other reasons to fork DefaultServer and DefaultClient to their own purposes.)