[Bro-Dev] Broker & CAF includes
jsiwek at illinois.edu
Mon Mar 21 09:43:08 PDT 2016
> On Mar 18, 2016, at 10:20 PM, Matthias Vallentin <vallentin at icir.org> wrote:
> During Broker refactoring, I noticed the following: all headers in
> broker/* include either standard library headers or Broker headers. This
> appears to be by design, which makes sense to me.
> As a library writer, one faces the tricky question of exposing headers
> from dependencies. For example, Broker currently has it's own
> broker::util::optional, which ships as a (now outdated) copy of the
> corresponding CAF source. I am inclined to change this copy to an
> include that points directly into CAF headers, with the following
> rationale: Broker already depends on CAF, and a system that has CAF
> installed always ships with CAF headers. (Strictly speaking, we're not
> copying the code of <vector> into broker either, but relying on it via
> an include.)
Just thought I’d share the logic behind the original decision. It was by design not to expose any CAF features directly in the public API of Broker, sort of as a proof that Broker’s interface would be sound enough to still work with arbitrary messaging back-ends/libraries besides CAF — i.e. treat the use of CAF an implementation detail.
Though, now I think this a case where including a CAF header might be an improvement and doesn’t defeat the original intention of treating CAF like an implementation detail since “optional” types aren’t a CAF-specific concept to begin with. The problem was just that they don’t come standard until c++17, I needed to get them some place, but the arbitrary rule I had for myself at the time said to generally not include CAF things in Broker’s public API. At the time, the risk of a copied version getting outdated seemed a lower priority to me than keeping Broker’s interface/design more simple/coherent in my head.
I think this is the only instance of this type of situation popping up when I was working on Broker and I can’t recall any other reasoning that led me handle it that way, so you’re proposed change looks good to me. Hope the explanation helps.
More information about the bro-dev