WordPoints / hooks-api

A basic API for hooking into user actions https://github.com/WordPoints/wordpoints/issues/321
GNU General Public License v2.0
0 stars 0 forks source link

Audit interfaces #19

Closed JDGrimes closed 8 years ago

JDGrimes commented 9 years ago

During development of the API, I've employed quite a few interfaces. They have been useful during development, but I'm not sure that all of them (any of them?) are really needed. We should consider the arguments for and against using interfaces in various contexts before releasing any of them. Especially since this would be the first time we've used any interfaces in WordPoints.

JDGrimes commented 8 years ago

At this point, I can see the benefit of interfaces and coding against an interface and not an implementation. The question is actually more whether we should be introducing some missing interfaces, not necessarily removing any.

JDGrimes commented 8 years ago

Our interfaces can basically be divided into two main categories:

  1. Complete object type — interfaces that encompass a whole basic set of methods that define a particular sort of object, like an entity or reaction. These are generally expected to be the only such interface implemented by a given class.
  2. Feature addition — interfaces that add just one or a few methods to an object that provide a particular feature. The objects are usually expected to be of a particular type (probably as defined by one of the interfaces of the above type), and may implement multiple such interfaces.

The latter are really necessary to the current design of the code. It is the former where things start to become less straightforward.

JDGrimes commented 8 years ago

First, because the first type of interface is meant to be the only type implemented by a class, we usually also provide a single abstract "bootstrap" class that includes some basic implementation of this interface, usually covering only a few of its methods. This means that we have both an interface and an abstract class, which doesn't seem entirely necessary. We could do essentially the same thing with just an abstract class, and then the interface would be unneeded. This SO question contains some interesting answers, especially this and this. From that later one:

Best practice is to use an interface to specify the contract and an abstract class as just one implementation thereof. That abstract class can fill in a lot of the boilerplate so you can create an implementation by just overriding what you need to or want to without forcing you to use a particular implementation.

Basically, the interface still serves a purpose, by allowing you to provide multiple different bootstrap implementations if you wanted to, without them having to extend from one another. Usually, however, we are providing only the most basic bootstrap, and so as a result we'll likely only ever have one abstract parent class.

JDGrimes commented 8 years ago

So it would be good to keep the interfaces around, because we don't want to depend upon a particular implementation. Unfortunately, I'm unsure whether the current arrangement where we have a single main implementation might lead us to unknowingly do this anyway.

JDGrimes commented 8 years ago

Something that we should consider though is that we've put constructors in some of the interfaces, but I don't think that is a good idea. Constructors are really an implementation detail, not a part of the contract.

JDGrimes commented 8 years ago

In some cases though, we are expecting the constructor to be a certain way in some places in our code. So perhaps in those cases the interface should be removed entirely in favor of an abstract class. On the other hand, I suppose that if the objects have use beyond that single application, the interface may be useful across those without having to be constructed in that same manner.

JDGrimes commented 8 years ago

I've decided to add interfaces for the reactor and extension classes. That left the args class as the only hooks class that didn't have an interface. I'm not sure that it really needs one so much, but I decide to give it one anyway.

JDGrimes commented 8 years ago

Really, I think we should just leave it at this. I see no reason to remove any interfaces at this point. I don't think that they can really do much harm other than requiring an extra file to be loaded, which does add some overhead. But all in all, I can't really see how any of these would be good to get rid of at this time. So I guess we'll just be stuck with them. In many ways they make for better forward-compatibility, I guess, anyway.