diff options
author | Aleksey Kladov <[email protected]> | 2021-02-12 15:10:16 +0000 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2021-02-12 15:10:16 +0000 |
commit | 799810eaaa2ac9aa3a597575270bd81ea0ef88b9 (patch) | |
tree | f99b8c6b35a1cd0bea638778102899101b4668e6 | |
parent | e0fc2af1184bed5af0a74276c261c79f685fa5d7 (diff) |
Document config pattern
-rw-r--r-- | docs/dev/style.md | 60 |
1 files changed, 60 insertions, 0 deletions
diff --git a/docs/dev/style.md b/docs/dev/style.md index 0482bc190..73ce59b87 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md | |||
@@ -368,6 +368,66 @@ impl ThingDoer { | |||
368 | 368 | ||
369 | **Rationale:** not bothering the caller with irrelevant details, not mixing user API with implementor API. | 369 | **Rationale:** not bothering the caller with irrelevant details, not mixing user API with implementor API. |
370 | 370 | ||
371 | ## Functions with many parameters | ||
372 | |||
373 | Avoid creating functions with many optional or boolean parameters. | ||
374 | Introduce a `Config` struct instead. | ||
375 | |||
376 | ```rust | ||
377 | // GOOD | ||
378 | pub struct AnnotationConfig { | ||
379 | pub binary_target: bool, | ||
380 | pub annotate_runnables: bool, | ||
381 | pub annotate_impls: bool, | ||
382 | } | ||
383 | |||
384 | pub fn annotations( | ||
385 | db: &RootDatabase, | ||
386 | file_id: FileId, | ||
387 | config: AnnotationConfig | ||
388 | ) -> Vec<Annotation> { | ||
389 | ... | ||
390 | } | ||
391 | |||
392 | // BAD | ||
393 | pub fn annotations( | ||
394 | db: &RootDatabase, | ||
395 | file_id: FileId, | ||
396 | binary_target: bool, | ||
397 | annotate_runnables: bool, | ||
398 | annotate_impls: bool, | ||
399 | ) -> Vec<Annotation> { | ||
400 | ... | ||
401 | } | ||
402 | ``` | ||
403 | |||
404 | **Rationale:** reducing churn. | ||
405 | If the function has many parameters, they most likely change frequently. | ||
406 | By packing them into a struct we protect all intermediary functions from changes. | ||
407 | |||
408 | Do not implement `Default` for the `Config` struct, the caller has more context to determine better defaults. | ||
409 | Do not store `Config` as a part of the `state`, pass it explicitly. | ||
410 | This gives more flexibility for the caller. | ||
411 | |||
412 | If there is variation not only in the input parameters, but in the return type as well, consider introducing a `Command` type. | ||
413 | |||
414 | ```rust | ||
415 | // MAYBE GOOD | ||
416 | pub struct Query { | ||
417 | pub name: String, | ||
418 | pub case_sensitive: bool, | ||
419 | } | ||
420 | |||
421 | impl Query { | ||
422 | pub fn all(self) -> Vec<Item> { ... } | ||
423 | pub fn first(self) -> Option<Item> { ... } | ||
424 | } | ||
425 | |||
426 | // MAYBE BAD | ||
427 | fn query_all(name: String, case_sensitive: bool) -> Vec<Item> { ... } | ||
428 | fn query_first(name: String, case_sensitive: bool) -> Option<Item> { ... } | ||
429 | ``` | ||
430 | |||
371 | ## Avoid Monomorphization | 431 | ## Avoid Monomorphization |
372 | 432 | ||
373 | Avoid making a lot of code type parametric, *especially* on the boundaries between crates. | 433 | Avoid making a lot of code type parametric, *especially* on the boundaries between crates. |