Commit b1d44ab3 authored by Alfredo Di Napoli's avatar Alfredo Di Napoli

review: add write-up about record-of-functions API

I think (but we have to try, we might realise is not the case) we
could make the library a bit more modular if we switch to a
records-of-functions API, which would allow for better encapsulation and
information hiding.
parent 95d7e390
Pipeline #6546 failed with stages
in 11 minutes and 16 seconds
......@@ -89,3 +89,96 @@ of [contra-tracer](https://hackage.haskell.org/package/contra-tracer-0.2.0.0/doc
broker, your mileage might vary) would gain an extra parameter (like a `Tracer` if using contra-tracer) and
then it's up to the caller to pick the concrete implementation for the logger, and the library would use
the opaque interface (that's the simplified TL;DR).
# Experiment with a record-of-functions API
After spending a bit of type with the library, I am wondering if the design would benefit from switching
away from a typeclass-based API in favour of a record-of-functions API. This stems from a few key observations:
## what is the record-of-functions approach
The TL;DR is that instead of having a typeclass:
```hs
class MessageBroker b a where
method1 :: ..
method2 :: ..
```
You have a record:
```hs
data BrokerAPI b a
= BrokerAPI
{ method1 :: ...
, method2 :: ...
}
```
You can typically combine this with typeclasses, which would be used for things like data families and
other "rigid" information (see later) which you want to lock in place once for all.
## typeclasses are rigid after instantiation
When we use a typeclass, really what we get is "bounded polymorphism" (as Simon Peyton Jones would put it):
rather than saying that our function is polymorphic over `a`, we say that our function can accept a reduced
set of types based on the constraint. This is fine, but it means that once we write an istance (for example
for `RedisBroker`) we cannot change any of the parameters dynamically if not by writing another instance,
which is not desirable. If we had the record of functions, we could do, for example:
```hs
-- just an example
data BrokerAPI b a
{ messageTimeout :: Int
}
later ..
updateTimeout :: Int -> BrokerAPI b a -> BrokerAPI b a
updateTimeout newTimeout ba = ba { messageTimeout = newTimeout }
```
In the same vein, tracers/loggers could be configured post-facto using records.
## It's hard to write things like `withBroker` without violating information hiding
At the moment, the `MessageBroker` class has two distinct methods for initialisation and deinitialisation.
As explained above in "Replace `initBroker` and `deinitBroker` with `withBroker`", this is not ideal because,
especially in the presence of exceptions, typically the cleanup function(s) are not called.
Unfortunately we _cannot_ prevent users from using `initBroker` and `deinitBroker` separately using the
typeclass style, because in order to implement a correct instance we must obviously expose those methods,
and there is no way to hide them; just importing `Async.Worker.Broker.Types` is enough to bring them
into scope (importing a module automatically imports typeclass and their instances in GHC) to break information
hiding.
If we had a record of functions, we could expose a `withBroker` function to our record, making sure that
in the _concrete_ module implementation we use the _private_ function for initialisation/deinitialisation.
## It could help with not violating information hiding for workers
Currently, we have this (simplified):
```hs
class MessageBroker b a where
data family Broker b a :: Type
...
```
And later, in the workers:
```hs
data State b a =
State { broker :: Broker b (Job a)
...
```
However, if you look at the concrete implementations for `MessageBroker`, we can see how the `Broker`
data family is _really_ the broker **internal** state (for example holding into a redis connection, etc).
However, the workers are given too much power, they are now coupled with the broker internals! They ideally
shouldn't need to know about that.
I haven't figured out all the details, but in a record-of-functions approach it should be possible to not
carry around the state at all, i.e. the `BrokerAPI` would be initialised with the internal state which will
be kept in the "closure" (so to speak) of the record, so that workers would be given access only to the
subset of public parts of the broker, without the need of knowing about the internals.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment