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