1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246
|
0) Summary
New code in aptitude should be written, wherever practical, with
fully abstract interfaces in .h files and hidden implementations in
.cc files.
1) Details
A standard abstract interface in aptitude looks like this:
class foo
{
public:
foo();
virtual ~foo();
virtual void method1(args...) = 0;
virtual void method2(args...) = 0;
// More methods ...
// Static constructor? (see below)
};
Note that both foo() and ~foo() are implemented in the .cc file.
foo() should be public so that any module can implement the foo
interface; this allows alternate implementations to be substituted
during testing.
Interfaces should contain only virtual methods, to aid multiple
inheritance, and they should generally derive only from other pure
virtual classes.
It might be tempting to derive from sigc::trackable, so signals can
be connected directly to interface methods. However, it is generally
better to either have the object connect itself (so the implementation
inherits trackable directly), or to connect the signal to a concrete
object that holds a strong reference to the interface. If you do
derive from trackable, please use virtual inheritance to avoid
unnecesary extra copies of "trackable" if an object implements several
interfaces.
1.1) Constructors
An abstract interface obviously can't be constructed.
Implementation classes should be hidden in .cc files and constructed
via either static methods on the interface, or free functions. Which
one is used depends on the relationship of the interface to the
implementation, as described below.
1.1.1) Single-implementation interfaces
Some interfaces in aptitude are only meant to have a single
implementation (except possibly for an implementation used just for
testing). These interfaces will expose their own creation via a
::create() static member. If there are several overloads of the
object's constructor, you should use different names for each static
creation routine: create_empty(), create_full(), etc.
The create() routine will construct an instance of a class that is
only declared in the corresponding .cc file.
For instance:
// In foo.h:
class foo
{
// ... as above ...
static boost::shared_ptr<foo> create();
};
// In foo.cc:
class foo_impl : public foo
{
public:
// Declare implementations of private methods in foo:
void method1(args...);
// ...
};
void foo_impl::method1(args...) { /* ... */ }
boost::shared_ptr<foo> create()
{
return make_shared<foo>();
}
1.1.2) Multi-implementation interfaces
Interfaces which are meant to have several implementations are
defined in their own .h files, but without any constructor methods.
Instead, they are constructed via free functions. For instance:
// In foo.h:
class foo
{
// ... as above ...
};
// In bar.h:
boost::shared_ptr<foo> create_bar();
// In bar.cc:
class bar : public foo
{
// Declare implementations of methods in foo.
};
// Define implementations of methods in foo.
boost::shared_ptr<foo> create_bar()
{
return make_shared<bar>();
}
1.2) Signals and slots
Instead of directly exposing signal members, signals should be
exposed via appropriately-named routines. This provides better
encapsulation (directly exposing sigc signals allows external code to
invoke them) and makes it easier to test code that uses the
interfaces.
class foo
{
/** \brief Register a callback to be invoked when the hidden
* signal is invoked.
*/
virtual sigc::connection connect_hidden(const sigc::slot<void> &slot) = 0;
};
Code in the src/qt tree can use the corresponding Qt idioms instead.
1.3) Mocks
By convention, mock implementations of interfaces should be placed
in a subdirectory mocks/ under the directory containing the .h of the
interface. The mock class has the same name as the interface, but
exists in a "mocks" namespace.
// In foo.h:
namespace aptitude
{
class foo
{
// Declarations of interface methods on foo.
};
}
// In mocks/foo.h:
namespace aptitude
{
namespace mocks
{
class foo : public aptitude::foo
{
// Mock out the methods of foo.
};
}
}
1.5) Global variables vs singleton classes vs static methods
Some functionality is truly global, and could be exposed via global
variables of class type, singleton classes with a hidden instance
variable, or static methods on a class.
Between these three approaches, singleton classes are preferred.
Using a singleton class behind an interface means that the
functionality can be swapped out, in test code if nowhere else. To
support this, it should be possible to pass in the interface as a
constructor parameter (although a convenience constructor that uses
the most common implementation of that interface is a good idea).
This is important because these modules represent access to system
functionality that might be awkward to use in test code. Existing
aptitude code violates this principle, with the result that it is
difficult to verify modules that use, for instance, the global
download cache or file queue.
1.6) Dependencies: constructor parameters vs created.
If an object accesses another object through an interface, it can
either take it as a constructor parameter or create it itself. Where
practical, it is preferred to pass it as a constructor parameter, to
make it possible to swap in different implementations of the
interface. However, it might be a good idea to provide a convenience
constructor that instantiates the most common implementations of the
interfaces that the class uses.
2) Motivation
Obviously, it is not necessary to use this many virtual interfaces.
There are a number of reasons that this approach is preferred:
2.1) Increasing testability
Unit tests are good. They let you verify your code up-front, and
they let you catch stealth changes to behavior early, before they
turn into bug reports. It's also difficult to write good unit
tests for complex pieces of code with many dependencies; you end
up testing the dependencies instead of the code you care about.
Breaking code into logically separated modules and dropping
abstraction layers between them makes it possible to separate
modules from the other code that they interact with. It also
makes it easy to test corner cases, particularly error
conditions, without having to generate synthetic system states
(for instance, what happens if the disk is full?), and it
provides a way of testing interactive code without requiring user
interaction (real or simulated).
2.2) Clear statements of internal interfaces
In a header file full of class definitions, with instance
variables, private methods, private member classes, and so on, it
can be tough to spot the actual interface methods. The coding
style described above places the public interface and (almost)
nothing else into the header file of a class.
I believe that this also leads to simpler interfaces, as it's
harder to hide poorly designed interface methods when they aren't
mixed with implementation concerns.
2.3) Fewer inter-header dependencies
Removing implementations from header files means that you don't
need to include the headers that are used by the implementation
from the header file. This can speed up compile times of client
code, in some cases significantly (for instance, when the
implementation requires some templates to be instantiated).
Furthermore, it means that modules using the interface don't have
to be recompiled just because the implementation changed, which
can speed up compiles significantly (old aptitude code was very
incestuous this way).
In extreme cases, this can break circular dependencies between
headers, although those tend to be signs that something e,se is
wrong anyway.
|