Skip to content

Commit e48c5ce

Browse files
Add public version of Firebase C++ style guide (#1740)
* Add new GCS service account key file. * Update C++ style guide to allow C++11 features and remove internal links * Update async operation pattern to *LastResult in style guide * Reformat STYLE_GUIDE.md to wrap at 80 characters * Reformat Jules.md to wrap at 80 characters * Revert formatting changes. * Fix filename. * Revert again. * Remove extra lines at the end. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent d299673 commit e48c5ce

File tree

2 files changed

+381
-0
lines changed

2 files changed

+381
-0
lines changed

AGENTS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,9 @@ API documentation.
306306

307307
## Coding Style
308308

309+
* **Firebase C++ Style Guide**: For specific C++ API design and coding
310+
conventions relevant to this SDK, refer to the
311+
[STYLE_GUIDE.md](STYLE_GUIDE.md).
309312
* **Google C++ Style Guide**: Adhere to the
310313
[Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html)
311314
as mentioned in `CONTRIBUTING.md`.

STYLE_GUIDE.md

Lines changed: 378 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,378 @@
1+
# C++ API Guidelines
2+
3+
**WIP** - *please feel free to improve*
4+
5+
Intended for Firebase APIs, but also applicable to any C++ or Game APIs.
6+
7+
# Code Style
8+
9+
Please comply with the
10+
[Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html)
11+
as much as possible. Refresh your memory of this document before you start :)
12+
13+
# C++ API Design
14+
15+
### Don't force any particular usage pattern upon the client.
16+
17+
C++ is a huge language, with a great variety of ways in which things can be
18+
done, compared to other languages. As a consequence, C++ projects can be very
19+
particular about what features of the language they use or don't use, how they
20+
represent their data, and structure their code.
21+
22+
An API that forces the use of a feature or structure the client doesn't use
23+
will be very unwelcome. A good API uses only the simplest common denominator
24+
data types and features, and will be useable by all. This can generally be
25+
done with minimal impact on your API’s simplicity, or at least should form
26+
the baseline API.
27+
28+
Examples of typical Do's and Don'ts:
29+
30+
* Don't force the use of a particular data structure to supply or receive
31+
data. Typical examples:
32+
* `std::vector<T>`: If the client doesn't have the data already in a
33+
`std::vector` (or only wants to use part of a vector), they are forced
34+
to copy/allocate a new one, and C++ programmers don't like unnecessary
35+
copies/allocations.
36+
Instead, your primary interface should always take a
37+
`(const T *, size_t)` instead. You can still supply an optional helper
38+
method that takes a `std::vector<T> &` and then calls the former if
39+
you anticipate it to be called very frequently.
40+
* `std::string`: Unlike Java, these things aren't pooled, they're mutable
41+
and copied. A common mistake is to take a `const std::string &`
42+
argument, which forces all callers that supply a `const char *` to go
43+
thru a strlen+malloc+copy that is possibly of no use to the callee.
44+
Prefer to take a `const char *` instead for things that are
45+
names/identifiers, especially if they possibly are compile-time
46+
constant. If you're unsetting a string property, prefer to pass
47+
nullptr rather than an empty string. (There are
48+
[COW implementations](https://en.wikipedia.org/wiki/Copy-on-write),
49+
but you can't rely on that).
50+
* `std::map<K,V>`: This is a costly data structure involving many
51+
allocations. If all you wanted is for the caller to supply a list of
52+
key/value pairs, take a `const char **` (yes, 2 stars!). Or
53+
`const SimpleStruct *` instead, which allows the user to create this
54+
data statically.
55+
* Per-product configuration should be accomplished using an options struct
56+
passed to the library's `firebase::<library>::Initialize` function.
57+
Default options should be provided by the options struct's default
58+
constructor. The `Initialize` function should be overloaded with a version
59+
that does not take the options struct (which is how the Google style guide
60+
prefers that we pass default parameters).
61+
62+
For example,
63+
64+
```c++
65+
struct LibraryOptions {
66+
LibraryOptions() : do_the_thing(false) {}
67+
68+
bool do_the_thing;
69+
};
70+
71+
InitResult Initialize(const App& app) {
72+
return Initialize(app, LibraryOptions());
73+
}
74+
InitResult Initialize(const App& app, const LibraryOptions& options);
75+
```
76+
77+
* Don't make your client responsible for data you allocate or take ownership
78+
of client data. Typical C++ APIs are written such that both the client
79+
and the library own their own memory, and they take full responsibility
80+
for managing it, regardless of what the other does. Any data exchanged is
81+
typically done through weak references and copies.
82+
An exception may be a file loading API where buffers exchanged may be
83+
really big. If you are going to pass ownership, make this super obvious
84+
in all comments and documentation (C++ programmers typically won't
85+
expect it), and document which function should be used to free the data
86+
(free, delete, or a custom one).
87+
Alternatively, a simple way to pass ownership of a large new buffer to
88+
the client is to ask the client to supply a std::string *, which you then
89+
resize(), and write directly into its owned memory. This somewhat
90+
violates the rule about use of std::string above, though.
91+
92+
* Don't use exceptions. This one is worth mentioning separately. Though
93+
exceptions are great in theory, in C++ hardly any libraries use them, and
94+
many code-bases disallow them entirely. They also require the use of RTTI
95+
which some environments turn off. Oh, yes, also don't use RTTI.
96+
97+
* Go easy on templates when possible. Yes, they make your code more general,
98+
but they also pull a lot of implementation detail into your API, lengthen
99+
compile times and bloat binaries. In C++ they are glorified macros, so
100+
they result in hard to understand errors, and can make correct use of
101+
your API harder to understand.
102+
103+
* Utilize C++11 features where appropriate. This project has adopted C++11,
104+
and features such as `std::unique_ptr`, `std::shared_ptr`,
105+
`std::make_unique`, and `std::move` are encouraged to improve code safety
106+
and readability. However, avoid features from C++14 or newer standards.
107+
108+
* Go easy on objectifying everything, and prefer value types. In languages
109+
like Java it is common to give each "concept" your API deals with its own
110+
class, such that methods on it have a nice home. In C++ this isn't
111+
always desirable, because objects need to be managed, stored and
112+
allocated, and you run into ownership/lifetime questions mentioned above.
113+
Instead:
114+
115+
* For simple data, prefer their management to happen in the parent
116+
class that owns them. Actions on them are methods in the parent. If
117+
at all possible, prefer not to refer to them by pointer/reference
118+
(which creates ownership and lifetime issues) but by index/id, or
119+
string if not performance sensitive (for example, when referring to
120+
file resources, since the cost of loading a file dwarfs the cost of
121+
a string lookup).
122+
123+
* If you must create objects, and objects are not heavyweight (only
124+
scalars and non-owned pointers), make use of these objects by value
125+
(return by value, receive by const reference). This makes ownership
126+
and lifetime management trivial and efficient.
127+
128+
* If at all possible, don't depend on external libraries. C++ compilation,
129+
linking, dependency management, testing (especially cross platform) are
130+
generally way harder than any other language. Every dependency is a
131+
potential source of build complexity, conflicts, efficiency issues, and
132+
in general more dependencies means less adoption.
133+
134+
* Don't pull in large libraries (e.g. BOOST) just for your
135+
convenience, especially if their use is exposed in headers.
136+
137+
* Only use external libraries that have hard to replicate essential
138+
functionality (e.g. compression, serialization, image loading,
139+
networking etc.). Make sure to only access them in implementation
140+
files.
141+
142+
* If possible, make a dependency optional, e.g. if what your API does
143+
benefits from compression, make the client responsible for doing so,
144+
or add an interface for it. Add sample glue code or an optional API
145+
for working with the external library that is by default off in the
146+
build files, and can be switched on if desired.
147+
148+
* Take cross-platform-ness seriously: design the API to work on ALL
149+
platforms even if you don't intend to supply implementations for all.
150+
Hide platform issues in the implementation. Don't ever include platform
151+
specific headers in your own headers. Have graceful fallback for
152+
platforms you don't support, such that some level of building / testing
153+
can happen anywhere.
154+
155+
* If your API is meant to be VERY widespread in use, VERY general, and very
156+
simple (e.g. a compression API), consider making at least the API (if
157+
not all of it) in C, as you'll reach an even wider audience. C has a
158+
more consistent ABI and is easier to access from a variety of systems /
159+
languages. This is especially useful if the library implementation is
160+
intended to be provided in binary.
161+
162+
* Be careful not to to use idioms from other languages that may be foreign
163+
to C++.
164+
165+
* An example of this is a "Builder" API (common in Java). Prefer to
166+
use regular constructors, with additional set_ methods for less
167+
common parameters if the number of them gets overwhelming.
168+
169+
* Do not expose your own UI to the user as part of an API. Give the
170+
developer the data to work with, and let them handle displaying it to
171+
the user in the way they see fit.
172+
173+
* Rare exceptions can be made to this rule on a case-by-case basis.
174+
For example, authentication libraries may need to display a sign-in
175+
UI for the user to enter their credentials. Your API may work with
176+
data owned by Google or by the user (e.g. the user's contacts) that
177+
we don't want to expose to the app; in those cases, it is
178+
appropriate to expose a UI (but to limit the scope of the UI to the
179+
minimum necessary).
180+
181+
* In these types of exceptional cases, the UI should be in an isolated
182+
component, separate from the rest of the API. We do allow UIs to be
183+
exposed to the user UI-specific libraries, e.g. FirebaseUI, which
184+
should be open-source so developers can apply any customizations
185+
they need.
186+
187+
# Game API Design
188+
189+
### Performance matters
190+
191+
Most of this is already encoded in C++ API design above, but it bears
192+
repeating: C++ game programmers can be more fanatic about performance than
193+
you expect.
194+
195+
It is easy to add a layer of usability on top of fast code, it is very
196+
hard to impossible to "add performance" to an API that has performance
197+
issues baked into its design.
198+
199+
### Don't rely on state persisting for longer than one frame.
200+
201+
Games have an interesting program structure very unlike apps or web pages:
202+
they do all processing (and rendering) of almost all functionality of the
203+
game within a *frame* (usually 1/60th of a second), and then start anew
204+
for the next frame.
205+
206+
It is common for all or part of the state of a game to be wiped out from
207+
one frame to the next (e.g when going into the menu, loading a new level,
208+
starting a cut-scene..).
209+
210+
The consequence of this is that the state kept between frames is the only
211+
record of what is currently going on, and that managing this state is a
212+
source of complexity, especially when part of it is reflected in external
213+
code:
214+
215+
* Prefer API design that is stateless, or if it is stateful, is so only
216+
within a frame (i.e. between the start of the frame and the start of
217+
the next one). This really simplifies the client's use of your API:
218+
they can't forget to "reset" your API's state whenever they change
219+
state themselves.
220+
221+
* Prefer not to use cross-frame callbacks at all (non-escaping callbacks
222+
are fine). Callbacks can be problematic in other contexts, but they're
223+
even more problematic in games. Since they will execute at a future
224+
time, there's no guarantee that the state that was there when the
225+
callback started will still be there. There's no easy way to robustly
226+
"clear" pending callbacks that don't make sense anymore when state
227+
changes. Instead, make your API based on *polling*.
228+
Yes, everyone learned in school that polling is bad because it uses
229+
CPU, but that's what games are based on anyway: they check a LOT of
230+
things every frame (and only at 60hz, which is very friendly compared
231+
to busy-wait polling). If your API can be in various states of a state
232+
machine (e.g. a networking based API), make sure the client can poll
233+
the state you're in. This can then easily be translated to user
234+
feedback.
235+
If you have to use asynchronous callbacks, see the section on async
236+
operations below.
237+
238+
* Be robust to the client needing to change state. If work done in your
239+
API involves multiple steps, and the client never gets to the end of
240+
those steps before starting a new sequence, don't be "stuck", but deal
241+
with this gracefully. If the game's state got reset, it will have no
242+
record of what it was doing before. Try to not make the client
243+
responsible for knowing what it was doing.
244+
245+
* Interaction with threading:
246+
247+
* If you are going to use threading at all, make sure the use of that
248+
is internal to the library, and any issues of thread-safety don't
249+
leak into the client. Allow what appears to be synchronous access
250+
to a possibly asynchronous implementation. If the asynchronous
251+
nature will be externally visible, see the section on async
252+
operations below.
253+
254+
* Games are typically hard to thread (since it’s hard to combine with
255+
its per-frame nature), so the client typically should have full
256+
control over it: it is often better to make a fully synchronous
257+
single-threaded library and leave threading it to the client. Do
258+
not try to make your API itself thread-safe, as your API is
259+
unlikely the threading boundary (if your client is threaded, use
260+
of your library is typically isolated to one thread, and they do
261+
not want to pay for synchronization cost they don't use).
262+
263+
* When you do spin up threads to reduce a workload, it is often a
264+
good idea to do that once per frame, as avoid the above mentioned
265+
state based problems, and while starting threads isn't cheap, you
266+
may find it not a problem to do 60x per second. Alternatively you
267+
can pool them, and make sure you have an explicit way to wait for
268+
their idleness at the end of a frame.
269+
270+
* Games typically use their own memory allocator (for efficiency, but
271+
also to be able to control and budget usage on memory constrained
272+
systems). For this reason, most game APIs tend to provide allocation
273+
hooks that will be used for all internal allocation. This is even more
274+
important if you wish to be able to transfer ownership of memory.
275+
276+
* Generally prefer solutions that are low on total memory usage. Games
277+
are always constrained on memory, and having your game be killed by
278+
the OS because the library you use has decided it is efficient to
279+
cache everything is problematic.
280+
281+
* Prefer to recompute values when possible.
282+
283+
* When you do cache, give the client control over total memory used
284+
for this purpose.
285+
286+
* Your memory usage should be predictable and ideally have no peaks.
287+
288+
# Async Operations
289+
290+
### Application Initiated Async Operations
291+
292+
* Use the Future / State Pattern.
293+
* Add a `*LastResult()` method for each async operation method to allow
294+
the caller to poll and not save state.
295+
296+
e.g.
297+
298+
```c++
299+
// Start async operation.
300+
Future<SignInWithCrendentialResult> SignInWithCredential(...);
301+
// Get the result of the pending / last async operation for the method.
302+
Future<SignInWithCrendentialResult> SignInWithCredentialLastResult();
303+
304+
Usage examples:
305+
// call and register callback
306+
auto& result = SignInWithCredential();
307+
result.set_callback([](result) { if (result == kComplete) { do_something_neat(); wake_up(); } });
308+
// wait
309+
310+
// call and poll #1 (saving result)
311+
auto& result = SignInWithCredential();
312+
while (result.value() != kComplete) {
313+
// wait
314+
}
315+
316+
// call and poll #2 (result stored in API)
317+
SignInWithCredential();
318+
while (SignInWithCredentialLastResult().value() != kComplete) {
319+
}
320+
```
321+
322+
### API Initiated Async Event Handling
323+
324+
* Follow the
325+
[listener / observer pattern](https://en.wikipedia.org/wiki/Observer_pattern)
326+
for API initiated (i.e where the caller doesn't initiate the event)
327+
async events.
328+
* Provide a queued interface to allow users to poll for events.
329+
330+
e.g.
331+
332+
```c++
333+
class GcmListener {
334+
public:
335+
virtual void OnDeletedMessage() {}
336+
virtual void OnMessageReceived(const MessageReceivedData* data) {}
337+
};
338+
339+
class GcmListenerQueue : private GcmListener {
340+
public:
341+
enum EventType {
342+
kEventTypeMessageDeleted,
343+
kEventTypeMessageReceived,
344+
};
345+
346+
struct Event {
347+
EventType type;
348+
MessageReceivedData data; // Set when type == kEventTypeMessageReceived
349+
};
350+
351+
// Returns true when an event is retrieved from the queue.
352+
bool PollEvent(Event *event);
353+
};
354+
355+
// Wait for callbacks
356+
class MyListener : public GcmListener {
357+
public:
358+
virtual void OnDeletedMessage() { /* do stuff */ }
359+
virtual void OnMessageReceived() { /* display message */ }
360+
};
361+
MyListener listener;
362+
gcm::Initialize(app, &listener);
363+
364+
// Poll
365+
GcmListenerQueue queued_listener;
366+
gcm::Initialize(app, &queued_listener);
367+
GcmListenerQueue::Event event;
368+
while (queued_listener(&event)) {
369+
switch (event.type) {
370+
case kEventTypeMessageDeleted:
371+
// do stuff
372+
break;
373+
case kEventTypeMessageReceived:
374+
// display event.data
375+
break;
376+
}
377+
}
378+
```

0 commit comments

Comments
 (0)