OLD | NEW |
---|---|
(Empty) | |
1 Coding Style Guidelines | |
2 ======================= | |
3 | |
4 These conventions have evolved over time. Some of the earlier code in both | |
5 projects doesn’t strictly adhere to the guidelines. However, as the code evolves | |
6 we hope to make the existing code conform to the guildelines. | |
7 | |
8 Files | |
9 ----- | |
10 | |
11 We use .cpp and .h as extensions for c++ source and header files. We use | |
12 foo_impl.h for headers with inline definitions for class foo. | |
13 | |
14 Headers that aren’t meant for public consumption should be placed in src | |
15 directories so that they aren’t in a client’s search path. | |
16 | |
17 We prefer to minimize includes. If forward declaring a name in a header is | |
18 sufficient then that is preferred to an include. | |
19 | |
20 Forward declarations and file includes should be in alphabetical order (but we | |
21 aren't very strict about it). | |
22 | |
23 Do not use #if/#ifdef before including "SkTypes.h" (directly or indirectly). | |
24 | |
25 We use spaces not tabs (4 of them). | |
26 | |
27 We use Unix style endlines (LF). | |
28 | |
29 We prefer no trailing whitespace but aren't very strict about it. | |
30 | |
31 We wrap lines at 100 columns unless it is excessively ugly (use your judgement). | |
32 The soft line length limit was changed from 80 to 100 columns in June 2012. Thus , | |
33 most files still adhere to the 80 column limit. It is not necessary or worth | |
34 significant effort to promote 80 column wrapped files to 100 columns. Please | |
35 don't willy-nilly insert longer lines in 80 column wrapped files. Either be | |
36 consistent with the surrounding code or, if you really feel the need, promote | |
37 the surrounding code to 100 column wrapping. | |
38 | |
39 Naming | |
40 ------ | |
41 | |
42 Both projects use a prefix to designate that they are Skia prefix for classes, | |
43 enums, structs, typedefs etc is Sk. Ganesh’s is Gr. Nested types should not be | |
44 prefixed. | |
45 | |
46 ~~~~ | |
jcgregorio
2015/01/21 17:55:39
<!--?prettify?-->
here and below
| |
47 class SkClass { | |
48 public: | |
49 class HelperClass { | |
50 ... | |
51 }; | |
52 }; | |
53 ~~~~ | |
54 | |
55 Data fields in structs, classes, unions begin with lowercase f and are then | |
56 camel capped. | |
57 | |
58 ~~~~ | |
59 struct GrCar { | |
60 ... | |
61 float fMilesDriven; | |
62 ... | |
63 }; | |
64 ~~~~ | |
65 | |
66 Globals variables are similar but prefixed with g and camel-capped | |
67 | |
68 ~~~~ | |
69 bool gLoggingEnabled | |
70 Local variables begin lowercases and are camel-capped. | |
71 | |
72 int herdCats(const Array& cats) { | |
73 int numCats = cats.count(); | |
74 } | |
75 ~~~~ | |
76 | |
77 Enum values are prefixed with k and have post fix that consists of an underscore | |
78 and singular name of the enum name. The enum itself should be singular for | |
79 exclusive values or plural for a bitfield. If a count is needed it is | |
80 k<singular enum name>Count and not be a member of the enum (see example): | |
81 | |
82 ~~~~ | |
83 enum SkPancakeType { | |
84 kBlueberry_PancakeType, | |
85 kPlain_PancakeType, | |
86 kChocolateChip_PancakeType, | |
87 | |
88 kLast_PancakeType = kChocolateChip_PancakeType | |
89 }; | |
90 | |
91 static const SkPancakeType kPancakeTypeCount = kLast_PancakeType + 1; | |
92 ~~~~ | |
93 | |
94 A bitfield: | |
95 | |
96 ~~~~ | |
97 enum SkSausageIngredientBits { | |
98 kFennel_SuasageIngredientBit = 0x1, | |
99 kBeef_SausageIngredientBit = 0x2 | |
100 }; | |
101 ~~~~ | |
102 | |
103 or: | |
104 | |
105 ~~~~ | |
106 enum SkMatrixFlags { | |
107 kTranslate_MatrixFlag = 0x1, | |
108 kRotate_MatrixFlag = 0x2 | |
109 }; | |
110 ~~~~ | |
111 | |
112 Exception: anonymous enums can be used to declare integral constants, e.g.: | |
113 | |
114 ~~~~ | |
115 enum { kFavoriteNumber = 7 }; | |
116 ~~~~ | |
117 | |
118 Macros are all caps with underscores between words. Macros that have greater | |
119 than file scope should be prefixed SK or GR. | |
120 | |
121 Static non-class functions in implementation files are lower case with | |
122 underscores separating words: | |
123 | |
124 ~~~~ | |
125 static inline bool tastes_like_chicken(Food food) { | |
126 return kIceCream_Food != food; | |
127 } | |
128 ~~~~ | |
129 | |
130 Externed functions or static class functions are camel-capped with an initial ca p: | |
131 | |
132 ~~~~ | |
133 bool SkIsOdd(int n); | |
134 | |
135 class SkFoo { | |
136 public: | |
137 static int FooInstanceCount(); | |
138 }; | |
139 ~~~~ | |
140 | |
141 Macros | |
142 ------ | |
143 | |
144 Ganesh macros that are GL-specific should be prefixed GR_GL. | |
145 | |
146 ~~~~ | |
147 #define GR_GL_TEXTURE0 0xdeadbeef | |
148 ~~~~ | |
149 | |
150 Ganesh prefers that macros are always defined and the use of #if MACRO rather th an | |
151 #ifdef MACRO. | |
152 | |
153 ~~~~ | |
154 #define GR_GO_SLOWER 0 | |
155 ... | |
156 #if GR_GO_SLOWER | |
157 Sleep(1000); | |
158 #endif | |
159 ~~~~ | |
160 | |
161 Skia tends to use #ifdef SK_MACRO for boolean flags. | |
162 | |
163 Braces | |
164 ------ | |
165 | |
166 Open braces don’t get a newline. “else” and “else if” appear on same line as | |
167 opening and closing braces unless preprocessor conditional compilation | |
168 interferes. Braces are always used with if, else, while, for, and do. | |
169 | |
170 ~~~~ | |
171 if (...) { | |
172 oneOrManyLines; | |
173 } | |
174 | |
175 if (...) { | |
176 oneOrManyLines; | |
177 } else if (...) { | |
178 oneOrManyLines; | |
179 } else { | |
180 oneOrManyLines; | |
181 } | |
182 | |
183 for (...) { | |
184 oneOrManyLines; | |
185 } | |
186 | |
187 while (...) { | |
188 oneOrManyLines; | |
189 } | |
190 | |
191 void function(...) { | |
192 oneOrManyLines; | |
193 } | |
194 | |
195 if (!error) { | |
196 proceed_as_usual(); | |
197 } | |
198 #if HANDLE_ERROR | |
199 else { | |
200 freak_out(); | |
201 } | |
202 #endif | |
203 ~~~~ | |
204 | |
205 Flow Control | |
206 ------------ | |
207 | |
208 There is a space between flow control words and parentheses and between | |
209 parentheses and braces: | |
210 | |
211 ~~~~ | |
212 while (...) { | |
213 } | |
214 | |
215 do { | |
216 } while(...); | |
217 | |
218 switch (...) { | |
219 ... | |
220 } | |
221 ~~~~ | |
222 | |
223 Cases and default in switch statements are indented from the switch. | |
224 | |
225 ~~~~ | |
226 switch (color) { | |
227 case kBlue: | |
228 ... | |
229 break; | |
230 case kGreen: | |
231 ... | |
232 break; | |
233 ... | |
234 default: | |
235 ... | |
236 break; | |
237 } | |
238 ~~~~ | |
239 | |
240 Fallthrough from one case to the next is commented unless it is trivial: | |
241 | |
242 ~~~~ | |
243 switch (recipe) { | |
244 ... | |
245 case kCheeseOmelette_Recipe: | |
246 ingredients |= kCheese_Ingredient; | |
247 // fallthrough | |
248 case kPlainOmelette_Recipe: | |
249 ingredients |= (kEgg_Ingredient | kMilk_Ingredient); | |
250 break; | |
251 ... | |
252 } | |
253 ~~~~ | |
254 | |
255 When a block is needed to declare variables within a case follow this pattern: | |
256 | |
257 ~~~~ | |
258 switch (filter) { | |
259 ... | |
260 case kGaussian_Filter: { | |
261 Bitmap srcCopy = src->makeCopy(); | |
262 ... | |
263 break; | |
264 } | |
265 ... | |
266 }; | |
267 ~~~~ | |
268 | |
269 Classes | |
270 ------- | |
271 | |
272 Unless there is a need for forward declaring something, class declarations | |
273 should be ordered public, protected, private. Each should be preceded by a | |
274 newline. Within each visibility section (public, private), fields should not be | |
275 intermixed with methods. | |
276 | |
277 ~~~~ | |
278 class SkFoo { | |
279 | |
280 public: | |
281 ... | |
282 | |
283 protected: | |
284 ... | |
285 | |
286 private: | |
287 SkBar fBar; | |
288 ... | |
289 | |
290 void barHelper(...); | |
291 ... | |
292 }; | |
293 ~~~~ | |
294 | |
295 Subclasses should have a private typedef of their super class called INHERITED: | |
296 | |
297 ~~~~ | |
298 class GrDillPickle : public GrPickle { | |
299 ... | |
300 private: | |
301 typedef GrPickle INHERITED; | |
302 }; | |
303 ~~~~ | |
304 | |
305 Virtual functions that are overridden in derived classes should use SK_OVERRIDE | |
306 (and not the override keyword). The virtual keyword can be omitted. | |
307 | |
308 ~~~~ | |
309 void myVirtual() SK_OVERRIDE { | |
310 } | |
311 ~~~~ | |
312 | |
313 This should be the last element of their private section, and all references to | |
314 base-class implementations of a virtual function should be explicitly qualified: | |
315 | |
316 ~~~~ | |
317 void myVirtual() SK_OVERRIDE { | |
318 ... | |
319 this->INHERITED::myVirtual(); | |
320 ... | |
321 } | |
322 ~~~~ | |
323 | |
324 As in the above example, derived classes that redefine virtual functions should | |
325 use SK_OVERRIDE to note that explicitly. | |
326 | |
327 Constructor initializers should be one per line, indented, with punctuation | |
328 placed before the initializer. This is a fairly new rule so much of the existing | |
329 code is non-conforming. Please fix as you go! | |
330 | |
331 ~~~~ | |
332 GrDillPickle::GrDillPickle() | |
333 : GrPickle() | |
334 , fSize(kDefaultPickleSize) { | |
335 ... | |
336 } | |
337 ~~~~ | |
338 | |
339 Constructors that take one argument should almost always be explicit, with | |
340 exceptions made only for the (rare) automatic compatibility class. | |
341 | |
342 ~~~~ | |
343 class Foo { | |
344 explicit Foo(int x); // Good. | |
345 Foo(float y); // Spooky implicit conversion from float to Foo. No n o no! | |
346 ... | |
347 }; | |
348 ~~~~ | |
349 | |
350 Method calls within method calls should be prefixed with dereference of the | |
351 'this' pointer. For example: | |
352 | |
353 ~~~~ | |
354 this->method(); | |
355 Memory Managemt | |
356 ~~~~ | |
357 | |
358 All memory allocation should be routed through SkNEW and its variants. These are | |
359 #defined in SkPostConfig.h, but the correct way to get access to the config | |
360 system is to #include SkTypes.h, which will allow external users of the library | |
361 to provide a custom memory manager or other adaptations. | |
362 | |
363 ~~~~ | |
364 SkNEW(type_name) | |
365 SkNEW_ARGS(type_name, args) | |
366 SkNEW_ARRAY(type_name, count) | |
367 SkNEW_PLACEMENT(buf, type_name) | |
368 SkNEW_PLACEMENT_ARGS(buf, type_name, args) | |
369 SkDELETE(obj) | |
370 SkDELETE_ARRAY(array) | |
371 ~~~~ | |
372 | |
373 Comparisons | |
374 ----------- | |
375 | |
376 We prefer that equality operators between lvalues and rvalues place the lvalue | |
377 on the right: | |
378 | |
379 ~~~~ | |
380 if (7 == luckyNumber) { | |
381 ... | |
382 } | |
383 ~~~~ | |
384 | |
385 However, inequality operators need not follow this rule: | |
386 | |
387 ~~~~ | |
388 if (count > 0) { | |
389 ... | |
390 } | |
391 ~~~~ | |
392 | |
393 Comments | |
394 | |
395 We use doxygen-style comments. | |
396 | |
397 For grouping or separators in an implementation file we use 80 slashes | |
398 | |
399 ~~~~ | |
400 void SkClassA::foo() { | |
401 ... | |
402 } | |
403 | |
404 //////////////////////////////////////////////////////////////// | |
405 | |
406 void SkClassB::bar() { | |
407 ... | |
408 } | |
409 ~~~~ | |
410 | |
411 Integer Types | |
412 ------------- | |
413 | |
414 We follow the Google C++ guide for ints and are slowly making older code conform to this | |
415 | |
416 (http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Integer_Types) | |
417 | |
418 Summary: Use int unless you have need a guarantee on the bit count, then use | |
419 stdint.h types (int32_t, etc). Assert that counts, etc are not negative instead | |
420 of using unsigned. Bitfields use uint32_t unless they have to be made shorter | |
421 for packing or performance reasons. | |
422 | |
423 NULL, 0 | |
424 ------- | |
425 | |
426 Use NULL for pointers, 0 for ints. We prefer explicit NULL comparisons when | |
427 checking for NULL pointers (as documentation): | |
428 | |
429 ~~~~ | |
430 if (NULL == x) { // slightly preferred over if (x) | |
431 ... | |
432 } | |
433 ~~~~ | |
434 | |
435 When checking non-NULL pointers explicit comparisons are not required because it | |
436 reads like a double negative: | |
437 | |
438 ~~~~ | |
439 if (x) { // slightly preferred over if (NULL != x) | |
440 ... | |
441 } | |
442 ~~~~ | |
443 | |
444 Returning structs | |
445 ----------------- | |
446 | |
447 If the desired behavior is for a function to return a struct, we prefer using a | |
448 struct as an output parameter | |
449 | |
450 ~~~~ | |
451 void modify_foo(SkFoo* foo) { | |
452 // Modify foo | |
453 } | |
454 ~~~~ | |
455 | |
456 Then the function can be called as followed: | |
457 | |
458 ~~~~ | |
459 SkFoo foo; | |
460 modify_foo(&foo); | |
461 ~~~~ | |
462 | |
463 This way, if return value optimization cannot be used there is no performance | |
464 hit. It also means that modify_foo can actually return a boolean for whether the | |
465 call was successful. In this case, initialization of foo can potentially be | |
466 skipped on failure (assuming the constructor for SkFoo does no initialization). | |
467 | |
468 ~~~~ | |
469 bool modify_foo(SkFoo* foo) { | |
470 if (some_condition) { | |
471 // Modify foo | |
472 return true; | |
473 } | |
474 // Leave foo unmodified | |
475 return false; | |
476 } | |
477 ~~~~ | |
478 | |
479 Function Parameters | |
480 ------------------- | |
481 | |
482 Mandatory constant object parameters are passed to functions as const references | |
483 if they are not retained by the receiving function. Optional constant object | |
484 parameters are passed to functions as const pointers. Objects that the called | |
485 function will retain, either directly or indirectly, are passed as pointers. | |
486 Variable (i.e. mutable) object parameters are passed to functions as pointers. | |
487 | |
488 ~~~~ | |
489 // src and paint are optional | |
490 void SkCanvas::drawBitmapRect(const SkBitmap& bitmap, const SkIRect* src, | |
491 const SkRect& dst, const SkPaint* paint = NULL); | |
492 // metrics is mutable (it is changed by the method) | |
493 SkScalar SkPaint::getFontMetrics(FontMetric* metrics, SkScalar scale) const; | |
494 // A reference to foo is retained by SkContainer | |
495 void SkContainer::insert(const SkFoo* foo); | |
496 ~~~~ | |
497 | |
498 If function arguments or parameters do not all fit on one line, they may be | |
499 lined up with the first parameter on the same line | |
500 | |
501 ~~~~ | |
502 void drawBitmapRect(const SkBitmap& bitmap, const SkRect& dst, | |
503 const SkPaint* paint = NULL) { | |
504 this->drawBitmapRectToRect(bitmap, NULL, dst, paint, | |
505 kNone_DrawBitmapRectFlag); | |
506 } | |
507 ~~~~ | |
508 | |
509 or placed on the next line indented eight spaces | |
510 | |
511 ~~~~ | |
512 void drawBitmapRect( | |
513 const SkBitmap& bitmap, const SkRect& dst, | |
514 const SkPaint* paint = NULL) { | |
515 this->drawBitmapRectToRect( | |
516 bitmap, NULL, dst, paint, kNone_DrawBitmapRectFlag); | |
517 } | |
518 ~~~~ | |
519 | |
520 Python | |
521 ------ | |
522 | |
523 Python code follows the [Google Python Style Guide](http://google-styleguide.goo glecode.com/svn/trunk/pyguide.html). | |
524 | |
OLD | NEW |