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